Opened 10 months ago
Last modified 10 months ago
#6811 new defect
Change 'ValueModification' message to be more efficient / remove ordering dependency
Reported by: | wraitii | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Alpha 27 |
Component: | Core engine | Keywords: | |
Cc: | Patch: |
Description
When a template-value should be modified by a modifier, from an aura, technology, status effect, etc., we currently send a 'MT_ValueModification' message from the ModifiersManager to the components.
This has several drawbacks:
- Most components are only interested in changes to themselves. However, any change is sent to all components. Many JS components subscribe, so we do a lot of redundant JS calls anytime a modifier changes. I haven't profiled this specifically, but it's not efficient. In particular, it ought to be somewhat noticeable at startup, since we send lots of messages to lots of entities.
- There is a subtle ordering issue that I think we currently avoid by chance only: BuildingAI.js, for example, listens for changes to
Attack
. However, if the message was sent to BuildingAI first, it wouldn't actually be able to see these changes, since it callsAttack.js
, which wouldn't necessarily be updated. - It technically is leaky. BuildingAI cares about changes to the interface, not to the actual values, which could be different things. The modifiers are related to components, in general, not interfaces (that's somewhat ill-defined to be fair). Ideally, we'd listen to changes to interface values here.
To fix the first issue, the simplest solution is to only send ValueModification to the target component. This requires some special handling but shouldn't be impossible, and can be optimised.
To fix the second issue (and make it possible to react to changes in other components), we should then probably create a new 'OnExternalModification' message that acts like the current ValueModification. It still wouldn't be massively efficient, but we have very few components that currently listen to changes in other components, so it should be an improvement already.
As an alternative, we could just split the message into one message per component. This also would require some changes to the code ideally, so that we have nice integration for mods. I am not sure of the performance implication of adding so many messages however, we might need to consider that impact on our ComponentManager. Note that even if we do this, we should probably inform the target component first.
I am not entirely sure what to do about the third point. Maybe it's fine enough.
Probably would be good to have before https://code.wildfiregames.com/D4991
Push back