User Details
- User Since
- Jan 8 2017, 4:17 PM (376 w, 4 d)
Feb 3 2019
I've looked at the code a bit more. There actually is a case when this function is called with a filter.
It happens in the "ATTACKING" timer function when a formation is ordered to attack another formation and only if the target formation is already in range.
The code incorrectly assumes that there are no more targets to find and calls FinishOrder() plus FindNewTargets().
From these 4 calls, only one passes a filter argument to the function. The other 3 can be ignored because of the check "if (filter ...". The condition after that has no impact when filter is undefined.
Jan 27 2019
Dec 3 2017
As I said, changes to Garbage Collection need a lot of testing. These unused functions were left in during the upgrade, but I'm not sure if using them makes sense anywhere.
In such a situation i'd also vote for removing the code. If there's ever a situation where triggering GC manually from scripts is necessary, then it should be quite easy to add it back (and a bit of work for testing, but you have that anyway). Leaving it in is probably more confusing than helpful.
Feb 5 2017
I'd like to review this, but at the moment it's a huge patch that changes many different and sometimes seemingly unrelated things. If you look at the history, it gets almost daily updates for more than a month now.
Would it be possible to pick out some of the changes and make separate patches for them?
Jan 28 2017
I've had another look at the patch. Looks good!
I've successfully tested on Windows with VS 2013 and on Ubuntu with GCC 4.8.4.
Jan 22 2017
Definitely an improvement! I'm wondering if it could still be extended more without using C++14 or newer features, but I couldn't come up with anything concrete yet. Maybe we could pass the function pointer type as a simple template type in some more functions instead of repeating the whole signature with the boost macros in call, callMethod and callMethodConst. Also I'm not sure if I recall completely why we need ScriptInterface_NativeMethodWrapper in addition to these call functions. One reason is to differentiate between the version with return value and the one with void, but there's probably a cleaner solution for that.