ITickable, deletion inside advanceTime

[lukaspj]

Adds a fix for when an ITickable object deletes itself inside the advanceTime procedure.
As discussed here

Kudos to David Wyand for this thread which explains the issue and helped me alot.

[lukaspj]

Copy of #435

[crabmusket]

I approve ;)

[DavidWyand-GG]

What is the special case at the end handling? If the last object in the list deletes itself then the size() method should return one less and the for() loop will dump out. Or have I missed something?

[lukaspj]

@DavidWyand-GG Hmm it was originally added because it kept iterating, but this may very well be fixed when I changed it to loop from 0 to size instead of from begin to end. Give me a sec to test and correct

Edit: removed the old commit and added a new one without special case.

[crabmusket]

Yeah, if you were still using begin() and end() it would have been necessary.

[lukaspj]

Charlie Patterson pointed out that getProcesslist()[i] in the if-clause may fail if the object is the last object in the list and it gets deleted so that i would be out of index bounds.
Somehow this haven't given me any issues yet, but immediate fix would be to add a i < getProcessList().size() clause to the if statement.
Charlie Patterson also suggest iterating backwards, but wouldn't this cause issues with process ordering?

[lukaspj]

Changed it but would you prefer a single commit or 2 commits?

[DavidWyand-GG]

@lukaspj If you could please make a new Pull Request with a single commit that would be great. Makes it easier to see what your final result is. Thanks!

[lukaspj]

@DavidWyand-GG With a little GitHub magic, a new pull request is unnecessary, now it's just a single commit!

[crabmusket]

Nice trick, I need to remember that one!

[DavidWyand-GG]

We took another look at this and I'm still not sure why you do the check for i here:

if(i < getProcessList().size() && iTick == getProcessList()[i])

If you only include the test for iTick and leave out the test for i what issues did you run into? Doesn't the for() loop already handle your out of range i?

[lukaspj]

If i=5 and getProcessList().size() = 6
And iTick deletes itself then getProcessList().size() = 5 and then when you in the if clause say:
iTick == getProcessList()[i] you will get an index out of bounds error.
The i < getProcessList().size() makes sure that there isn't an index out of bounds error if the last object in the list deletes itself.

[DavidWyand-GG]

This has been replaced with pull request #529 to both extend the support for deleted objects with ITickable processing, and to format the code to fit T3D norms.