Use signals to dispose of the children of Viewports in RGSS1

The weak references method was throwing errors if the object id had already been recycled

Author: WaywardHeart

binding/disposable-binding.h

@@ -26,100 +26,6 @@
 #include "binding-util.h"
 #include "graphics.h"
 
-/* 'Children' are disposables that are disposed together
- * with their parent. Currently this is only used by Viewport
- * in RGSS1. */
-inline void
-disposableAddChild(VALUE disp, VALUE child)
-{
-    GFX_LOCK;
-    if (NIL_P(disp) || NIL_P(child)) {
-        return;
-    }
-    
-    VALUE objID = rb_obj_id(child);
-    
-	VALUE children = rb_iv_get(disp, "children");
-    
-    bool exists = false;
-
-	if (NIL_P(children))
-	{
-		children = rb_ary_new();
-		rb_iv_set(disp, "children", children);
-	}
-    else {
-        exists = RTEST(rb_funcall(children, rb_intern("include?"), 1, objID));
-    }
-
-    if (!exists) {
-        rb_ary_push(children, objID);
-        VALUE objectspace = rb_const_get(rb_cObject, rb_intern("ObjectSpace"));
-        VALUE method = rb_funcall(disp, rb_intern("method"), 1, rb_id2sym(rb_intern("_sprite_finalizer")));
-        rb_funcall(objectspace, rb_intern("define_finalizer"), 2, child, method);
-    }
-    GFX_UNLOCK;
-}
-
-inline void
-disposableRemoveChild(VALUE disp, VALUE child)
-{
-    GFX_LOCK;
-    if (NIL_P(disp) || NIL_P(child)) {
-        return;
-    }
-    
-    VALUE objID = rb_obj_id(child);
-    
-    VALUE children = rb_iv_get(disp, "children");
-    if (NIL_P(children))
-        return;
-    
-    VALUE index = rb_funcall(children, rb_intern("index"), 1, objID);
-    if (NIL_P(index))
-        return;
-    
-    rb_funcall(children, rb_intern("delete_at"), 1, index);
-    GFX_UNLOCK;
-}
-
-inline void
-disposableForgetChild(VALUE disp, VALUE child)
-{
-    VALUE children = rb_iv_get(disp, "children");
-    
-    if (NIL_P(children)) {
-        return;
-    }
-    
-    VALUE index = rb_funcall(children, rb_intern("index"), 1, child);
-    if (NIL_P(index)) {
-        return;
-    }
-    
-    rb_funcall(children, rb_intern("delete_at"), 1, index);
-}
-
-inline void
-disposableDisposeChildren(VALUE disp)
-{
-	VALUE children = rb_iv_get(disp, "children");
-
-	if (NIL_P(children))
-		return;
-
-    for (long i = 0; i < RARRAY_LEN(children); ++i) {
-        int state;
-        rb_protect([](VALUE args){
-            VALUE objectspace = rb_const_get(rb_cObject, rb_intern("ObjectSpace"));
-            VALUE ref = rb_funcall(objectspace, rb_intern("_id2ref"), 1, args);
-            rb_funcall(ref, rb_intern("_mkxp_dispose_alias"), 0);
-            return Qnil;
-        }, rb_ary_entry(children, i), &state);
-    }
-		//rb_funcall2(rb_ary_entry(children, i), dispFun, 0, 0);
-}
-
 template<class C>
 RB_METHOD(disposableDispose)
 {
@@ -134,9 +40,6 @@ RB_METHOD(disposableDispose)
 	if (d->isDisposed())
 		return Qnil;
 
-	if (rgssVer == 1)
-		disposableDisposeChildren(self);
-
     GFX_LOCK;
 	d->dispose();
     GFX_UNLOCK;
@@ -162,11 +65,6 @@ static void disposableBindingInit(VALUE klass)
 {
 	_rb_define_method(klass, "dispose", disposableDispose<C>);
 	_rb_define_method(klass, "disposed?", disposableIsDisposed<C>);
-
-	/* Make sure we always have access to the original method, even
-	 * if it is overridden by user scripts */
-	if (rgssVer == 1)
-		rb_define_alias(klass, "_mkxp_dispose_alias", "dispose");
 }
 
 template<class C>

binding/viewport-binding.cpp

@@ -68,29 +68,10 @@ RB_METHOD(viewportInitialize) {
     wrapProperty(self, &v->getColor(), "color", ColorType);
     wrapProperty(self, &v->getTone(), "tone", ToneType);
 
-    /* 'elements' holds all SceneElements that become children
-     * of this viewport, so we can dispose them when the viewport
-     * is disposed */
-    rb_iv_set(self, "elements", rb_ary_new());
     GFX_UNLOCK;
     return self;
 }
 
-RB_METHOD(viewportSpriteFinalize)
-{
-    RB_UNUSED_PARAM;
-    
-    VALUE objectid;
-    
-    rb_get_args(argc, argv, "o", &objectid RB_ARG_END);
-    
-    if (rgssVer == 1) {
-        disposableForgetChild(self, objectid);
-    }
-    
-    return Qnil;
-}
-
 DEF_GFX_PROP_OBJ_VAL(Viewport, Rect, Rect, "rect")
 DEF_GFX_PROP_OBJ_VAL(Viewport, Color, Color, "color")
 DEF_GFX_PROP_OBJ_VAL(Viewport, Tone, Tone, "tone")
@@ -111,7 +92,6 @@ void viewportBindingInit() {
     sceneElementBindingInit<Viewport>(klass);
 
     _rb_define_method(klass, "initialize", viewportInitialize);
-    _rb_define_method(klass, "_sprite_finalizer", viewportSpriteFinalize);
 
     INIT_PROP_BIND(Viewport, Rect, "rect");
     INIT_PROP_BIND(Viewport, OX, "ox");

binding/viewportelement-binding.h

@@ -55,12 +55,6 @@ RB_METHOD(viewportElementSetViewport)
 
 	if (!NIL_P(viewportObj))
 		viewport = getPrivateDataCheck<Viewport>(viewportObj, ViewportType);
-    
-    if (rgssVer == 1) {
-        VALUE vp = viewportElementGetViewport<C>(0, 0, self);
-        disposableRemoveChild(vp, self);
-        disposableAddChild(viewportObj, self);
-    }
 
 	GFX_GUARD_EXC( ve->setViewport(viewport); );
 
@@ -82,9 +76,6 @@ viewportElementInitialize(int argc, VALUE *argv, VALUE self)
 	if (!NIL_P(viewportObj))
 	{
 		viewport = getPrivateDataCheck<Viewport>(viewportObj, ViewportType);
-
-		if (rgssVer == 1)
-			disposableAddChild(viewportObj, self);
 	}
 
     GFX_LOCK;

src/display/viewport.cpp

@@ -232,7 +232,10 @@ void Viewport::releaseResources()
 ViewportElement::ViewportElement(Viewport *viewport, int z, int spriteY)
     : SceneElement(viewport ? *viewport : *shState->screen(), z, spriteY),
       m_viewport(viewport)
-{}
+{
+	if (rgssVer == 1 && viewport)
+		viewportDispCon = viewport->wasDisposed.connect(&ViewportElement::viewportElementDisposal, this);
+}
 
 Viewport *ViewportElement::getViewport() const
 {
@@ -242,7 +245,25 @@ Viewport *ViewportElement::getViewport() const
 void ViewportElement::setViewport(Viewport *viewport)
 {
 	m_viewport = viewport;
+	
+	viewportDispCon.disconnect();
+	if (rgssVer == 1 && viewport)
+		viewportDispCon = viewport->wasDisposed.connect(&ViewportElement::viewportElementDisposal, this);
+	
 	setScene(viewport ? *viewport : *shState->screen());
 	onViewportChange();
 	onGeometryChange(scene->getGeometry());
 }
+
+void ViewportElement::viewportElementDisposal()
+{
+	viewportDispCon.disconnect();
+	Disposable *self = dynamic_cast<Disposable*>(this);
+	if(self != nullptr)
+		self->dispose();
+}
+
+ViewportElement::~ViewportElement()
+{
+	viewportDispCon.disconnect();
+}

src/display/viewport.h

@@ -71,6 +71,7 @@ class ViewportElement : public SceneElement
 {
 public:
 	ViewportElement(Viewport *viewport = 0, int z = 0, int spriteY = 0);
+	~ViewportElement();
 
 	DECL_ATTR( Viewport,  Viewport* )
 
@@ -79,6 +80,9 @@ class ViewportElement : public SceneElement
 
 private:
 	Viewport *m_viewport;
+	sigslot::connection viewportDispCon;
+	sigslot::connection viewportElementDispCon;
+	void viewportElementDisposal();
 };
 
 #endif // VIEWPORT_H