Skip to content

Conversation

@Bit-Crust
Copy link
Contributor

Previously, scripts were recorded with an unordered map from script name to enabled state, however this doesn't guarantee maintained order, causing script failure when behavior in the Create function (or elsewhere) of one script depends on the behavior of other scripts, which may be scheduled in certain circumstances to occur too late. An ordered map wouldn't work because it requires a comparison function, when the order needs to be determined by insertion order. It seems like it may as well be handled with a vector of script names, while maintaining the unordered map of script name to enabled state. However, each script name is recorded twice here (this is also what I have implemented, it works). It seems somehow less than ideal.

…ordered map from script name to enabled state, however this doesn't guarantee maintained order, causing script failure. An ordered map wouldn't work because it requires a comparison function, when the order needs to be determined by insertion order. It seems like it may as well be handled with a vector of script names, while maintaining the unordered map of script name to enabled state. However, each script name is recorded twice there (this is also what I have implemented, it works). Probably the best solution would be to just keep a vector of script name and a vector of corresponding enabled states, however the efficiency of searching goes from log of size to linear with position, which seems bad, but nothing should ever have enough scripts on it to be a problem, is what I imagine. Feels like I'm missing the optimal solution.
@pawnishoovy
Copy link
Contributor

don't know enough c++ to comment on the technicalities but can confirm this keeps script order in-game by repeating steps in my mods that would previously break the order (and now they don't). @Causeless ?

@pawnishoovy pawnishoovy requested a review from Causeless January 25, 2025 15:26
@HeliumAnt
Copy link
Contributor

HeliumAnt commented Feb 12, 2025

You could do a std::vector<std::pair<std::string, bool>> to replace the unordered map, which would mean the initial code could stay mostly the same, pretty much, except for the insertion (try_emplace becomes emplace_back) and the search which would need to be a find_if with string compare lambda.

Might technically even be slightly faster as well, std::unordered_map performance is notoriously abysmal. Especially at these small entry numbers

@Causeless Causeless added this pull request to the merge queue Feb 19, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 19, 2025
@Causeless Causeless added this pull request to the merge queue Feb 20, 2025
Merged via the queue into development with commit 0ff075c Feb 20, 2025
4 checks passed
@Causeless Causeless deleted the ordered-scripts branch February 20, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants