TheXTech
Threaded FindWldStars
One user has complained that the level scan process in FindWldStars takes too long and delays them starting an episode. To fix this, Norgel suggested that we should run FindWldStars in a separate thread during the world map, instead of at the end of the world load process (and I fully agree with this suggestion).
But as currently implemented, FindWldStars is not thread safe. It accesses a DirListCI, which internally includes an std::map. It also modifies WorldLevel_t::save_infos, which are non-atomic, and thus could lead to corruption if a gamesave is in process while a world level's save info is being modified. We clearly need to add a lock, but I'm not sure what portions of the code these locks should guard.
That means, these things will require a mutex thing (SDL2 has it as well as std::mutex, however, the last one doesn't work everywhere, and SDL_mutex works just fine even on Wii [I implemented its support via OGC's API]). And so, on single-threaded buils, that screen will be used.
I think, there is also a way to optimize the thing: dump the list of files from the DirListCI and use the dumped copy instead of polling the DirListCI every time (but, that would work as long as no levels in sub-directories was being placed which will require to poll the DirListCI again to retrieve sub-directorie's content). :thinking:
dump the list of files from the DirListCI and use the dumped copy instead of polling the DirListCI every time
That seems like a good solution. If we'll do this, it's equally cheap to copy-initialize another DirListCI on the heap and pass that copy to the thread (instead of using the global one, which is not thread-safe). We should confirm that the copy-initialization doesn't do any extra filesystem operations, since FindWldStars is often called when all of the save_info is already initialized and no filesystem access is needed.
Then the only lock needed would be a global lock on the WorldLevels' save_info. (assuming that the filesystem operations used by PGE-FL are thread-safe)
I guess, I can quickly make the "duplicate()" call that can make a full copy of the self to use it inside threads. What do you think?
I made a workaround for this that only scans levels where the max stars are requested to be shown by the WLDX file. This has the side effect that medal counts will only be shown after the level is visited for the first time. However, my understanding is that this is the situation for X2 also, so I think it acceptable for now.