Fix memory corruption from instance_deactivate_object()

[sorlok]

A while ago, someone fixed the massive memory leak in iterator's object_basic constructor. This is a good thing! However, it lead to this issue in instance_deactivate_object:

//Paraphrased:
enigma::iterator it = enigma::fetch_inst_iter_by_int(obj);
enigma::instance_deactivated_list.insert(inode_pair((*it)->id,it.it));

//In fetch_inst_iter_by_int():
iterator ret = iterator(a->second->inst);
return a != instance_list.end() ? iterator(a->second->inst) : iterator();

//In iterator::iterator(object_basic*):
iterator::iterator(object_basic* ob) : temp_iter(ob, NULL, NULL), it(&temp_iter) {}

//And, finally, in the iterator class definition:
struct iterator {
  inst_iter temp_iter;
  struct inst_iter* it;
};

So, correct me if I am wrong (which is entirely possible!), but fetch_inst_iter_by_int() is returning a temporary as a pointer (the it(&temp_iter)), which will eventually blow up. I noticed this occurring somewhat-predictably in both Iji and An Untitled Story.

To fix this, I noted that any object_basic* is manually unlinked before being inserted into instance_deactivated_list, so I turned instance_deactivated_list into a map of object_basic* rather than inst_iter*.

This led to a bug with instance_iter_queue_for_destroy(ENOBJ_ITER_me) queueing up the wrong object, so I just replaced it with instance_iter_queue_for_destroy(this), which seems more correct anyway. I am not sure how ENOBJ_ITER_me gets corrupted (I know it happens if a destroy() event creates and links a new object of a different type), but I suspect the instance system in general.

[RobertBColton]

@sorlok I bugged Josh to get the massive var leak fixed and we spent a couple of hours patching it when we did in c3debac

I can pull your changes and test them out tomorrow for regressions, I'll let @JoshDreamland also weigh in.

[sorlok]

Yes, let's hear from Josh on this too. I wasn't around for the leak patch, so you two should definitely discuss this.

[EnigmaBot]

(Posted by egofree on the ENIGMA forums)

This is a little bit off topic, but as you working on the instance_deactivate_object function, do you think you could see also the problem with the self object ? If you use instance_deactivate_object(self), it will deactivate not only the current object but all child objects. This is not the correct behavior in GM. I reported the problem here http://enigma-dev.org/tracker/index.php?id=754

[EnigmaBot]

(Posted by egofree on the ENIGMA forums)

correction: it will deactivate not the child objects, but objects of the same type.

[sorlok]

Sure, I can have a look. Do you have a small test case that demonstrates this?

[RobertBColton]

@sorlok I have pulled the branch and everything seems to work fine, I tested 1945, FPS Demo, Project Mario and a few other games, but I don't have a unit test for instance activation deactivation to test and I have no idea to what degree ENIGMA's activation/deactivation did work. Do you happen to have a unit test for activation/deactivation on hand?

[JoshDreamland]

Oh, holy shit. Yes, that old behavior was incorrect. It only ever worked because we were leaking memory like a sieve. I have no problem with this patch whatsoever. I can't say with any certainty that this won't lead to other regressions, but this is a step in the right direction.

[EnigmaBot]

(Posted by egofree on the ENIGMA forums)

sorlok:
Here is a test case : https://dl.dropboxusercontent.com/u/29802501/object_deactivate.egm

When the project starts, you have a black square which goes to the right and collides with a red square. In the red square object, i've a collision event with the following line : instance_deactivate_object(self); As you can see, when it collides with the red square, all red squares disappear. This is not the case in GM studio. (You can export the project as a gmx project and test it with GM Studio).

[EnigmaBot]

(Posted by egofree on the ENIGMA forums)

Sorlok:
I don't know if you would be interested to fix this bug also, but there is a problem with the instance_activate_object function. It is not working correctly with objects inheritance. If you are interested, here is a test case : https://dl.dropboxusercontent.com/u/29802501/object_activate.egm
There are two objects : black_square and red_square which inherits from the object square. In the room 'creation code', i put the following lines : instance_deactivate_object(square); instance_activate_object(square);. It deactivates correctly the objects, but it doesn't activate the objects again. This is not the case in GM Studio.

[sorlok]

@RobertBColton I don't really have a test for this; I was testing the behavior on An Untitled Story, since that was where I could consistently produce it. I wrote a test case, but it only crashed half the time.

[RobertBColton]

@sorlok Well, I guess it's not important it's still a step in the right direction and from what I can tell it doesn't noticeably break anything, and I'm certain you will fix any regressions that pop up. Josh also seems to be satisfied, so I am going to merge this, nice work as always!