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 calls Attack.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

Change History (1)

comment:1 by Stan, 10 months ago

Milestone: Alpha 28Alpha 27

Push back

Note: See TracTickets for help on using tickets.