[PATCH] Cache technology modifiers based on both name and value

[O.Davoodi]

Currently we only cache the technology modifiers based on the name of the value. In the current code, if the value changes, it is not registered in the TechManager and the older value is returned.

It has already broken the "TARDIS" cheat, which changes the player's maximum population. It can also cause problems in future (for example, when implementing a stat changer trigger effect).

To fix this, we can make the game cache the modifier based on both name and value.

[O.Davoodi]

A patch to fix the issue

[leper]

I'm quite certain that delete this.modificationCache[valueName][ent]; will not work as it should be cacheBase instead of ent.

[O.Davoodi]

New version

[O.Davoodi]

Fixed it.

[O.Davoodi]

leper: do you have any other suggestions?

[leper]

I'm not sure if clearModificationCache() works correctly, as I think the break could lead to missing some modifications. If you have two modifications with different curValues clearing the cache will just remove the first cached value, so I'd opt for removing the break.

I'm not really sure if the current way is the best way of handling this, but I can't think of a better way currently and if it works (and is properly tested) it should be committed IMO.

While we're at it we could add some tests for the TechnologyManager.

[O.Davoodi]

I'm not sure if clearModificationCache() works correctly, as I think the break could lead to missing some modifications. If you have two modifications with different curValues clearing the cache will just remove the first cached value, so I'd opt for removing the break.

This is the case if the older modifications are actually not removed, because I don't see any way for us to have two caches with different curValue's unless we actually have not cleared the cache somewhere else.

While we're at it we could add some tests for the TechnologyManager.

I'm afraid I don't know how to do it. I have seen the test folders but don't know what do they do exactly.

[sanderd17]

I think my patch is a bit cleaner, conceptually. As it doesn't allow one ent with two different old values to be cached.

[leper]

In 14205:

Cache technology modifiers based on original value. Patch from sanderd17 based on work by Spahbod. Fixes #2045.