We-the-People-civ4col-mod / Mod

This is the repository where the mod resides.
90 stars 37 forks source link

Domestic advisor is slow to open #61

Open Nightinggale opened 6 years ago

Nightinggale commented 6 years ago

The code calculates all page headers for each page each time the window is opened. This causes lag.

The file in question is:

Assets\Python\Screens\CvDomesticAdvisor.py

Change the code to not calculate the headers until the page in question is opened. This will distribute the calculations to not cause one massive lag at startup.

Some windows are set up on first call and then just hide widgets instead of deleting. This means it opens a lot faster next time because the game don't have to calculate the widgets again. Investigate if this can be done with the Domestic Advisor.

Figure out if storing headers between advisor sessions can cause issues if the player changed screen resolution in the meantime. Possibly store screen resolution in the window class instance and when opening, if the stored values differs from the current ones, discard the headers and start over. If they are identical, try to reuse.

See if opened page can be stored between advisor sessions.

Since this is GUI only and mainly about lag, this is cosmetic and has no influence on game data.

Nightinggale commented 6 years ago

170 changed how the yield demands are calculated for the domestic market and now all yields are calculated in one call. To aid whenever a single yield is needed (like help text for a single yield), the old function is still present, but it is a frontend to the new, meaning it calculates everything, returns the requested yield and discards the rest.

The DA is however not updated. It loops yields and requests one at a time, meaning it calculates all yields X times where X is the number of columns. It works, but it's obviously not efficient.

A proper and clean fix would require python exposure of the C++ classes JustInTimeArray and EnumTypeCacheArray. Adding python support for those would be really nice for multiple situations, but it's certainly not a 5 minute job to add those.

Nightinggale commented 6 years ago

I just had an idea about this. We can use the same concept as with the xml gui editor where each screen is a class. This allows the init code to make an array of class instances, but only init is called for each when the window is opened. The actual setup for each class will then be postponed until draw() is called for each. This means if you open to check say where you have tools, you will not spend time setting up say the native advisor. While this will not speed up the code itself, it will spread out the slowdown to when the user clicks on the buttons and just a little each time, meaning the lag will not be as easy to notice as it is right now.

Having each screen in a class will also make the code cleaner. Each class should have standard functions like input and draw, meaning the main class can call those in the array of class instances without considering which one it is, kind of like objective programming, but without a shared base class.

The init code should take a reference to the main class as this will allow shared code in the main class, like setting up yield column headers as those are used for multiple screens.

This approach is used in the xml gui editor with great success. Converting from one big class to multiple small classes have made the entire code much more readable. The conversion to this format is however not 100% complete as WTP suddenly took priority.

Nightinggale commented 5 years ago

I think if the goal is to improve performance, the answer isn't a python reorganization. Instead add a new Cy class in the DLL to handle all the calculations. The new python code then becomes "stupid" to a level where it just draws whatever the dll tells it to draw to such a degree that say buildings and yields might share the same python code.

Making the DLL do the calculations is usually the best option whenever there is a performance issue.