Fixed RemoveCallback skipping over later callbacks

[Guantol-Lemat]

Now if a callback is meant to be removed, whilst the callback list is being iterated over, it is instead replaced with a null function. Once the callback list has finished running (and no other instances of that callback list are being iterated over) all the null functions are removed from the list.

[ConnorForan]

Hi there - thank you for your interest in contributing to REPENTOGON

I'm the one who implemented the reworked callback handling. I'm also the one who the github issue that this PR fixes, https://github.com/TeamREPENTOGON/REPENTOGON/issues/337, is currently assigned to.

We had similar thoughts previously on how to address this issue, namely in disabling callbacks instead of removing them and then coming back later to clean them up. Ran into the same question that you did in terms of how to handle multiple instances of a callback being active at once. Though I decided to set it aside at the time to focus on the reworks.

I definitely understand your approach here, by continually tracking whether or not there are active running instances of any given callback. However, I have a few concerns with the potential for overhead and the added complexity to general callback handling that comes with doing that tracking. It's probably negligible performance wise, but the callback handling is a sensitive area of the code, so forgive me for being a bit cautious. Though the added complexity of accurately maintaining the callback tracking in the future, for each variation of callback handling logic, and the whole "customNullFunctions"/"DefaultNullFunction" thing I'm a bit unsure about. Admittedly it feels like it could be a bit much just to solve one admittedly niche issue that has long existed in the vanilla API.

I very much appreciate the effort you put into this fix, but I think I would prefer to close this PR. I'm hesitant to mess around too much in the callback handling logic more until after the next update for the sake of stability. I also feel like I would prefer to implement the fix for this issue myself, ideally something with minimal footprint. I have a couple thoughts on that front, especially after seeing your approach.

I do hope that this does not come off as disrespectful. Again, this part of the code is sensitive and reasonably young still, so I'll admit I'm being overly cautious for now. Sorry, but thank you again for your interest & effort.

[Guantol-Lemat]

Oh, no worries, I understand. I hope I've at least provided some insight into a possible solution.

I had also come up with other possible solutions which I did not propose, mainly because I thought they would raise the complexity and overhead a bit too much.

Check if a Callback can be Safely Removed:

This is essentially a more complex version of the solution I proposed. This one involved checking wether or not the callback that had to be removed was later on in the list compared to the one that was currently being executed, and if it was it could be removed otherwise it would have to be scheduled.

This one I discarded because it would cause more overhead during callback preparation and execution, as I would need to track where each instance of a callback list iteration was, then find out which one was farthest down the list, and then I would only remove a callback if it came after the farthest one, otherwise I would need to schedule it's removal for later (which I also did using null functions). I would also need to remove or schedule the callback in the PARAM or COMMON list right away, as I couldn't determine the position relative to the farthest function without adding more overhead during callback preparation.

Overall I thought this solution would have added too much complexity to the general callback handling process and so I discarded it midway.

Iterate over a DeepCopy of the Callback List:

This one is the most straightforward, as it didn't add any complexity to the process itself. However I chose to discard this one prematurely because of the overhead caused by having to copy the entire callback list before iteration.