AntiLag measures

[florianessl]

I wasn't sure if I should create a pull request yet, but as this is a good place for discussion I'm ready to share it..:

I tried optimizing the code responsible for map event page refreshes (MEPR):

This heavy-handed approach to refreshing the map state is a performance problem for games that do many switch & variable operations in the background, particularly for maps with a ton of events.
So much, that there a several patches already for RPG_RT itself, that try to mitigate this, by either disabling the refreshing logic on demand (AntiLagSwitch) or changing the behavior entirely (AntiLagFast, PF-Version of Maniacs).
I myself had this performance issue on two areas in my game project: Both the overworld and hub area have > 1500 events. They work fine on modern PCs, but the game will really lag when using 10x speed and it is to be expected that other, less up-to-date platforms will have similar issues. (Also, trying to play these maps with debug flags is a challenge)

Simple implementation for AntiLagSwitch
First commit is a simple implementation for the "AntiLagSwitch" patch which just does that: Disable MEPR entirely if a particular switch is ON. This itself already should provide a conscious dev with a very powerful way to control this behavior and do complex operations in the game interpreter without having to worry about impact on performance.
The switch only affects map events right now, but from what I read on the Makerpendium wiki, the original version would also disable the refreshing of common events.

Implemented experimental "map event caches" to drastically increase p…
The second commit is an attempt to revise the MEPR logic without going the extreme route (Maniacs PF) which would create incompatibilities with existing games. This was in my head for months now, since I first went over this particular part of the interpreter code. Why is it neccessary to go over every single map event & every single event page, every time a single switch or variable is changed?
This commit refactors the logic to only set the refresh flag, whenever a switch or variable is changed that actually is used by an Game_Event on the current map. So every time a map is loaded, after the events list has been populated, two unordered maps will also get initialized:

• events_cache_by_switch
• events_cache_by_variable

These contain all map events that would actually be needed to be considered for MEPR, organized by switch-id & variable-id respectively. It could even by condensed to just unordered sets containing the ids, since the MapEventCache structure and its vector of events isn't actually in use right now. Whenever a single switch-id or variable-id is changed, the refresh flag is only set if a key is found for that particular id. The actual code responsible for refreshing the map event pages remains the same but is called much less frequently.
This drastically improves performance in my case (without even having to set the AntiLagSwitch) and many existing games that contain some more complex scripts might benefit from such a change.

I went through all code that contains switch/variable operations which operate on a single element (range operations still set the refresh flag every time) and changed the calls to one of:

• SetNeedRefreshForSwitchChange
• SetNeedRefreshForVariableChange

Some parts that would be used less frequently, as well instances of "SetNeedRefresh" calls on Item/Timer/Party updates, etc. I considered negligible, so they remain as they were.

I haven't done any actual performance metrics yet but can already say, the results are very noticeable..
Also I rarely dabble in C++, so this could likely be implemented in a much better way. At the moment, I also don't have any assessment on whether the additional lookups for the swich/variable ids might cause some (minor) adverse effect on performance in other types of scenarios.

[Ghabry]

About the map event cache: Your observation is correct and I had the same idea around half a year ago: Ghabry@aed9775

But my solution consisted of tracking all writes to variables in a seperate vector and then polling that vector everytime. It solves the problem with "range" but was very ugly so I decided to not push it further.

Your idea is better here: You simply did not bother to add it for ranges which makes the code much easier and no need for slow polling. 👍 I think this code is the way to go.

[florianessl]

Concercing these MapEventCache structs which I mentioned could also be just replaced with unordered sets:

I do have some sort of Prefetcher in mind that would work hand in hand with the Interpreter and tries to predict which game resources to load ahead of time, to reduce any sort of lag that could otherwise occur when e.g. graphics are requested for immediate use in a scene. As many sprite updates in typical game scenes are triggered by change of switch/variable values, the current draft for this prefetching mechanism makes use of this structure.

Many creators of older games never bothered with designing CharSets in a way that would make sense for use in the Interpreter. So a character gets killed & their sprite is changed to some animation that is part of a completely different file. Then the scene goes on and more & more files are requested right at the moment that they are needed. If the IO lags for whatever reason this becomes very noticeable. I noticed similar things in laggy I/O scenarios with all sorts of graphics as well as with Music & sounds. Some old games even do animations by making use of a series of pictures in a row.

This will probably take a while but I aim to prepare development environments for different platforms (I do still have my old Wii, 3DS, PS2 & PS3 lying around somewhere...) to see what makes sense in terms of the design.
(A more aggressive or a more soft, predictive approach for pre-fetching resources..?)

[Ghabry]

Imo the code is already pretty solid.

I have some suggestions for code changes (2 commits).

https://github.com/Ghabry/easyrpg-player/commits/AntiLag

The first commit gets rid of the strtol parsing and makes all patch options an individual option (and preserves --patch for backwards compat). Also documentation updates. (all the --no-patch-* options are currently pretty useless, maybe become useful later...)

The second commit:

• std::unique_ptr is only useful when the initialisation should be delayed or the object is very large. In short I simply removed it.
• The change in line 453 fixes a bug that is even in master: The time_variable never triggered a refresh.
• Changed MapEventCache to store the event id instead of a copy of the event. Saves memory and you can still fetch them from the database via this ID. (this stuff is currently a stub but I will keep it so you can extend it further later as discussed in your commit)
• Fixed indent of MapUpdateAsyncContext together with indent of MapEventCache (already wrong in master)

---

Cherry-Pick command for reference if you want to grab the commits as-is:

git remote add ghabry https://github.com/ghabry/easyrpg-player
git fetch ghabry
git cherry-pick COMMITHASH  <- repeat for both

[Ghabry]

not for committing but this code here is great to see how many unnecessary refreshes were skipped :)

void Game_Map::SetNeedRefreshForSwitchChange(int switch_id) {
	if (need_refresh)
		return;
	if (events_cache_by_switch.find(switch_id) != events_cache_by_switch.end())
		SetNeedRefresh(true);
	else
		Output::Debug("{} no refresh", switch_id);
}

void Game_Map::SetNeedRefreshForVarChange(int var_id) {
	if (need_refresh)
		return;
	if (events_cache_by_variable.find(var_id) != events_cache_by_variable.end())
		SetNeedRefresh(true);
	else
		Output::Debug("{} no refresh", var_id);
}

[Ghabry]

As there was no reaction to my comments yet I'll apply them on Friday to this PR if there is not further feedback.