chore: less unnecessary stuff on gui events (don't squash)

[winex]

best to be merged after #300 (or #308 at least) don't squash, pls

ok, i've reworked this, it's now not fix-desync, but "do less on gui events", but should still help against possible desyncs. didn't test it in multiplayer as #245 is not yet fixed fully

1. no need to call Consumption::updateNeeds() from gui as it is already called from:

   • CityPlanner::initializeCity()
   • City::growCitizenCount()
   • on_entity_destroyed_event (after #300 is merged)

2. added:

   • Constants.CITY_STATS_LIFETIME_TICKS -- 30 seconds should be fine
   • city.stats.providedUpdateTick

3. little cleanup of control.lua by Constants reusal

now, Consumption.updateProvidedAmounts() doesn't get called on each gui event for a city, only after its lifetime has passed. full code path is still a bit complex for my understanding, so i hope this doesn't conflict with original design

please review if i did this correctly, i.e. game.tick check

tests

• [ ] TEST: get no desyncs in 24 hours with this fix applied - then it can be merged asap. failed
• [ ] TODO: TEST: gui events setting 1 global var not to cause any desyncs again postponed
• [ ] TODO: TEST: multiplayer with 3+ players crazy clicking town-hall overview buttons ;) just to prove this PR should be merged as well, haha

todo

we need to trigger updateProvidedAmounts() in deferred mode on gui events, update can be called from control.lua's main loops. such vars are usually called dirty flag until it is cleared like that:

-- example code, not in PR
for _, city in ipairs(global.tycoon_cities or {}) do
    -- set on gui events
    if city.needsProvidedUpdate then
        Consumption.updateProvidedAmounts(city)
    end
    -- WARN: must only be cleared here
    city.needsProvidedUpdate = false
end

old, already-merged, etc...

note: if some event calls updateNeeds() in similar "async" mode, like on_construction...() or on_built() or w/e <-- should be looked into this first if desyncs happen again... ~~funcs such as growCitizenCount() should not be written in handlers, so i reused better one from City:: class. yeah i know, that was a big refactoring - i've dug into some history...~~ btw, i didn't test Util.findCityByTownHallUnitNumber() - i hope assert() works in on_gui_opened() :)

[winex]

damn! with this applied, i just got client-2 desync (gui opened for some minutes: TH->Basic->Construction Materials) client-1 log has this, desync in 10 seconds after startConstruction() was successful"

17723.113 Script @__tycoon__/city.lua:829: startConstruction(): coordinates: {x = 13, y = 6}
17734.212 Info ClientMultiplayerManager.cpp:747: updateTick(1558806) received player (2) desynced

after looking some more into this - i see many more calls from gui events, which we probably need to move out: in Consumption:: getSupplyLevels() calls updateProvidedAmounts(). but we want separate calls for this because of gui ...or maybe i'm just missing something easier! :(

...converting PR to draft as it needs some more work!

[winex]

so, these gui events might not caused desyncs, see https://github.com/bahrmichael/factorio-tycoon/issues/245#issuecomment-2006917728 and #289

but i still think this should be merged in some way as this is sooo hard to track! let's keep it here for some time until i get more data...

[winex]

ok, finally returned to this PR and reworked it. looks ready. this can also improve performance on top of #303, can you re-profile it the same way?

[bahrmichael]

can you re-profile it the same way?

Can't do that here unfortunately, as it needs a lot of clicking which is not as predictable as a loop that's just running all the time.

The change looks good to me, and it showed no bugs in testing 👍