Whistle Stop Factories


Spawns massive furnaces and assembly machines around the map to make the game all about building trains to connect them

Content
3 years ago
0.16 - 1.1
2.90K
Manufacturing

g Multi-second freeze when opening the GUI of a whistle factory

5 years ago
(updated 5 years ago)

Added WSF to a large modpack and opening the GUI for a WSF building incurs multi-second freezing, it appears to be because it is iterating the entire recipes list to hide/unhide as appropriate. This is incurring both the cost of the iteration of the entire massive recipe list via for _, recipe in pairs(force.recipes) do in addition to a costly string comparison via if string.sub(recipe.name, -4)=="-big". Instead it might be good to cache all the recipes that need to be toggled as such upon loading the game, then you can just iterate that list instead, which will be significantly shorter, in addition it means the string check on every recipe item does not need to be performed (the recipe names can be added to the list as the recipes are being generated after all), which should pretty well reduce the lag to just about nothing when opening one of their GUI's). :-)

5 years ago

Multi-second? Every time you open the GUI? That is downright unplayable, and I agree needs to be addressed.

And nice work taking the initiative. Looks like you found the problem and have highlighted probably the best solution.

So yes, the main time I'd calculate the cache would be during config_change which is an event that happens during game load when mods have been removed/added/updated, so in other words perfect for this. Even then, multi-second still seems a little absurd, and I should probably consider trying to optimize it further.

One issue though is with newly unlocked recipes. I can run an event every time a tech is researched to add the corresponding "-big" recipes to the cache. There are a couple edge cases that are problems though. Suppose a recipe is unlocked through something besides tech research? I think seablock uses this, though my mod can't really work with seablock anyway. Or what if a recipe is unlocked by a script after the tech, similar to what I'd be doing. If my script ran first, it wouldn't be able to see the other newly unlocked recipe and wouldn't include it.

So there are some advantages of calculating it on every GUI open... but obviously we need to address the unplayability first. So I guess I may have to settle for just doing my best to unlock the normal recipes opened by techs and ignore the edge case weird ones.

Maybe if I get a more efficient version I could run it every 10 minutes or something as a double check, though that'd still be an annoying game freeze.

I'll have to give it some more thought lately, but yes, this is something I absolutely intend to figure out how to fix for you.

5 years ago
(updated 5 years ago)

Multi-second? Every time you open the GUI?

Yep, large modpack though.

That is downright unplayable, and I agree needs to be addressed.

Meh, I'm dealing with it, they aren't exactly opened very often (it's not long enough to drop players in MP). ^.^

One issue though is with newly unlocked recipes. I can run an event every time a tech is researched to add the corresponding "-big" recipes to the cache. There are a couple edge cases that are problems though. Suppose a recipe is unlocked through something besides tech research? I think seablock uses this, though my mod can't really work with seablock anyway. Or what if a recipe is unlocked by a script after the tech, similar to what I'd be doing. If my script ran first, it wouldn't be able to see the other newly unlocked recipe and wouldn't include it.

You could still include the check for those, right now the code is:

local function enablebigrecipe(force)
    for _, recipe in pairs(force.recipes) do
        if string.sub(recipe.name, -4)=="-big"
            and inlist(recipe.category, {"big-smelting", "big-recipe", "big-chem", "big-refinery"}) 
            and force.recipes[string.sub(recipe.name,1,-5)]
            and force.recipes[string.sub(recipe.name,1,-5)].enabled
            and not force.recipes[string.sub(recipe.name,1,-5)].hidden then 
                recipe.enabled=true
        end 
    end
end

Just replacing it as such (pseudo-fillers):

local function enablebigrecipe(force)
    for _, entry in pairs(global.recipe_cache) do
        if force.recipes[entry.original_recipe]
            and force.recipes[entry.original_recipe].enabled
            and not force.recipes[entry.original_recipe].hidden then 
                force.recipes[entry.big_recipe].enabled=true
        end 
    end
end

And appropriate for the disable. I wouldn't think this would be a big cost considering it's still a restricted iterative set and all access does not require memory allocation so it 'should' still be fast (no string work and no iteration of entire recipe set saves the big costs).

(Assuming the recipe cache is a simple table of 2 named entries, but you could make that even faster by making them indexed by integer lookups, or even faster yet by just encoding them as the key/value directly, like the big recipe name as the key and the small recipe name as the value or vice-versa would be the fastest, adjusting the for loop as necessary to iterate over named table entries efficiently if using that style.)

5 years ago

That makes sense. I'll still have to recalculate the list during config_change, but that would be the ONLY time I'd have to do it. I could just ignore tech changes. Even then, there might be better ways to calculate the list, like checking the crafting category first might save time. I have done absolutely 0 profiling in lua or factorio, so I'm not exactly sure how I could test various approaches for timing, but can at least use my knowledge from python programming on generally poor approaches.

This approach would also require pretty minimal changes. I wouldn't even need to include that in migration or update code, just calculate the cache on the first config_change that is called, which will be called first thing when you update to the version I make with this feature.

Okay, I'll give this approach a try and you'll have to let me know how much it saves on the WSF GUI opening performance.

For the version of the code in my config_change to make the cache, now that I think about it, I may just have to reorder my checks:

if string.sub(recipe.name, -4)=="-big" and inlist(recipe.category, {"big-smelting", "big-recipe", "big-chem", "big-refinery"}) and force.recipes[string.sub(recipe.name,1,-5)] and force.recipes[string.sub(recipe.name,1,-5)].enabled and not force.recipes[string.sub(recipe.name,1,-5)].hidden then

You said string.sub(recipe.name, -4)=="-big" was expensive, but I started with it because I thought it'd be fairly cheap. Would inlist(recipe.category, {"big-smelting", "big-recipe", "big-chem", "big-refinery"}) be cheaper? Or even just a straight (category=="big-smelting" or category="..)?

5 years ago

String comparisons in general are not too cheap. They can early-out easy enough so as long as the prefix is often different then it's better, but successes still mean every byte was checked.

The string.sub is expensive because it means it has to allocate memory for it, which is then compared, which is then tossed out, it's the worst bit by far and removing it would be the biggest gain possible.

5 years ago

Well that part is actually completely unnecessary then. The check is pointless because if it is in one of those 4 recipe.categories it will automatically end with "-big". I just figured a string compare of size 4 would be faster than the string compares for the full recipe categories. I should also try to arrange the categories by frequency... so probably big-recipe, big-smelting, big-chem, then big-refinery.

5 years ago

0.17.57 introduced a new recipe hiding feature that I have now implemented (please upgrade to WSF 0.2.0). Now there isn't even a need for ANY looping. All the recipes are just tagged with a property that hides them from the crafting menu.

New response