[PATCH] Better way to handle visibilities in the simulation

[Itms]

My work on #599 allowed me to work on the visibility systems (fog-of-war, line of sight, etc.), and currently, a large part of the visibility logic is handled by the RangeManager system component, especially in ​this function.

As a consequence, scripted components can't influence the visibility of an entity. Mods, but also planned features for the main game (like Iberian camouflaged units), would need a way to influence this visibility. Also, the way I plan to tackle #2710 needs it.

I propose to separate this logic into a Visibility component. Several things could benefit from this change.

I think it would be better to do changes in separate patches/commits, to avoid complexity and possible regressions. So I set this ticket as a task, and I will use it to track the changes. Remaining planned changes:

• Create the Visibility component
• Find a way to remove the hardcoded and duplicated VIS_FOO in the scripted components Fogging, Mirage and Visibility
• Move template items GetRetainInFog and AlwaysVisible from Vision to Visibility
• Update component documentations (RangeManager, Visibility), including code comments
• Fix #2710 with the new system
• Improve #958 with the new system if possible (see ​this TODO)
• For fun, implement camouflaging for some units

[Itms]

In 15925:

Add a new Visibility component that will eventually allow scripted components and mods to influence an entity's visibility.

This first commit only refactors code and does not add any new feature.

Refs #2913 (see this ticket for more information about the change)

[Itms]

In 15939:

Fix actors not retained in FoW after r15925.
Refs #2913.

[Itms]

In 16022:

Move template items GetRetainInFog and AlwaysVisible from Vision to Visibility.

Refs #2913

[Itms]

Here is a patch to use the cached visibility (introduced for mirages) for any entity. This would reduce the number of computations of the visibility, which is currently huge, but no profiling was done yet. I will come up with some data but I post the patch for review in the meantime.

[Itms]

Committed above patch in r16166.

[Itms]

Here is a patch moving back the computation of visibilities to C++. Now, the scripted component is called only if necessary.

As a use case of the scripted part, I fixed the TODO remaining from #958.

However, a style improvement would be handling properly the empty properties of scripted components in templates. For instance, currently Decay has an <Inactive/> component, well handled in C++, but I was forced to write <Preview>true</Preview> in my case to make the code work. This must be fixed before committing.

[Itms]

Updated the template thing after getting Philip's input on IRC.

[Itms]

In 16175:

Clean up redundancies in templates for the Visibility component, refs #2913

[Itms]

Fixed the patch, which didn't actually work.

[Itms]

Hi Ben, could you take a look at the latest patch, which works around a code you wrote? I'd like to commit it asap, probably tomorrow. Thanks in advance :)

[Itms]

In 16248:

Move back the computation of most of the visibilities from JS to engine, to improve performance.

Also fix the remaining TODO left by #958, as a use case of the scripted Visibility component.

Refs #2913, #958.

[Itms]

Attached patch reduces drastically the number of mirages by making fogging conditional: entities will be miraged only if their health/resource amount is reduced, or if they have a non-gaia owner.

It also cleans out the code in some parts, and fixes the invisible animals problem and the missing status bars for mirages.

I'll commit it on Friday evening, so please give feedback :)

[mimo]

Some small comments from a quick look at the patch which looks nice

For gaia, we should also activate mirage if the health increases (i.e. gaia tower being repaired), so the activation should also be done in Health.prototype.Increase.

In StatusBar, we could remove useless parentheses: lines 106-108, 112-114 and 118-120 + those added in the patch

[Itms]

Ah yeah, missed these two things, thanks :) I can't fix them right now though.

In the meantime, could you or Yves run the tracelogger on a patched version to see what impact it would have? That would be great!

[Yves]

The tracelogger isn't the best tool for measuring here. Reducing the number of entities can have an impact on many different parts of the engine. I'd try with profiler1 and a preconfigured AI match (-autostart argument). Could you wait until Saturday evening before committing? I can have a look, but not before the weekend

[Yves]

I've done some counting of entities, but haven't tired to measured performance differences.

Normally, the map is currently revealed for AI players, so this patch doesn't have an effect on them and can't be tested. I've disabled revealing of the map for testing. This also means it will only improve performance for human players and especially if there are more human players (multiplayer).

I've used this command to test on a map with a lot of miraged entities (trees).

./pyrogenesis -autostart="random/deep_forest" -autostart-players=4 -autostart-size=256 -autostart-ai=1:petra -autostart-ai=2:petra -autostart-ai=3:petra -autostart-ai=4:petra

I've waited until the first few attacks are over (by this time, all players should have "scouted" quite a bit) and counted the total number of entities.

Original Start of the match: 5856 entities After first series of attacks: 12795 entities

Patch Start of the match: 5519 entities After first series of attacks: 5734 entities

As you see, it dramatically reduces the number of entities and I guess it also has quite an impact on performance. Very clever idea for this patch!

[Itms]

Addressed mimo's style comments.

[Itms]

In 16281:

Reduce drastically the number of mirages by making fogging conditional: entities will be miraged only if their health/resource amount is modified, or if they have a non-gaia owner.

Fixes the animals hidden in the FoW, and adds the missing status bars for mirages.

Also small cleanup of the code.

Refs #2913

[Yves]

A match on deep forest with 3 AIs. Very laagy at the end during a large attack (up to 30 ms/frame in UpdateVisibilityData)

[Yves]

One problem I see is that m_DirtyVisibility is not player-specific. It will mark all tiles as dirty that have changed their visibility state for any player and then updates the visibility of that tile for all players. I've added some printing of the percentage of tiles that are dirty in each run of UpdateVisibilityData. It starts at around 3-4% and goes up to more than 20% later in game. I think it would be faster with caching per player and only updating the bits of the visibility flag for the player whose vision on that tile changed.

[Yves]

A quick and ugly hack to reduce the number of IsActive() calls to JS along with some changes used for profiling

[Yves]

Performance measurement of the hack. I've used the autostart options to setup a match with 4 AI players on deep_forest with SimRate set to 8 (ignore the absolute values and be careful with the interpretation of that data). However, the difference between the two UpdateVisibilityData lines is surprisingly big.

[Itms]

Make the dirty visibility flag a bit more clever.

[Itms]

Results of the clever_dirty patch

[Itms]

In 16328:

Make the visibility cache a bit more clever, by making LoS tiles as dirty separately for each player.
It is necessary to rely on shared los masks, else some visibility updates will be missing.

Refs #2913, see this ticket for a performance graph.

[Itms]

This patch moves the Visibility activation logic to the range manager for performance. I upload in a few minutes a graph comparing it with r16329.

[Itms]

Tweaks to the previous patch

[Yves]

Well done! Measurements are way better now (with both patches) and I've played a whole game without any laag caused by UpdateVisibilityData. I'd say it's fast enough now.

Something else: Is it intended that you can't build in the fog of war now? I didn't check when that was introduced. If it's intended, that seems to be OK for me (avoids some difficult situations like when players try to build on an obstructed place and the problem of cheating that arises when we give early feedback if building is possible or not).

[Yves]

Updated graph (still with a sim rate of 8)

[Yves]

In 16337:

Move visibility component activation check to C++. Patch by Itms.

This improves performance quite a lot because it avoids a huge number of calls from C++ to JS. Check the ticket for performance measurements.
Refs #2913

[Lionkanzen]

Can be used to get  spies units into the gameplay?, if is possibly modding feature to this

"As a consequence, scripted components can't influence the visibility of an entity. Mods, but also planned features for the main game (like Iberian camouflaged units), would need a way to influence this visibility. Also, the way I plan to tackle #2710 needs it."

[Itms]

I don't see a clean way to remove the hardcoded values in the scripts (it would lead to more hacks and less flexibility).

So everything in the list is completed. Camouflaging is #3177.