Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser and processor fixes #166

Merged
merged 6 commits into from
Dec 15, 2024
Merged

Conversation

MichaelMakesGames
Copy link
Collaborator

This fixes 3 issues:

  • memory leak caused by _get_or_add_shared_description Potential Memory Leak? #163
  • parsing failure caused by initial "nan" being interpretted as Not a Number Parse Error #164
  • 500 error serving the event ledger, caused by missing leader (reported in Stellaris Modding Den discord)

Also bumps up the Stellaris version number in the mod descriptor (already updated on workshop)

@@ -69,7 +77,7 @@ <h2 class="eventlist_header">{{title[object] | safe}}</h2>
<div class="eventdescription">
<span class="dateblock">{% if event["is_active"] %}(A){% endif %} {{event["start_date"]}}{% if event["end_date"] != null %} - {{event["end_date"]}}{%endif%}:</span>
<span class="eventtext">
{{ event["leader"].leader_class.capitalize() }} {{ links[event["leader"]] | safe }} ruled the {{ links[event["country"]] | safe}}
{{ render_leader(event, "An unknown leader") }} ruled the {{ links[event["country"]] | safe}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple things I'm not sure about in this template language is, one, why pass "An unknown leader" when the fallback of "an unknown leader" exists? is this passing a specific fallback string to capitalized "an"?

two, does it still work with the seemingly keyword argument in the macro vs the positional argument in these calls?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's just capitalization
The macro syntax is similar to functions in python: you can pass arguments by position or name

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'm not sure I remembered one could pass keyword arguments by position, but it's not too complicated here at least

@@ -72,6 +72,7 @@ def process_gamestate(self, game_id: str, gamestate_dict: Dict[str, Any]):
)
if config.CONFIG.debug_mode or isinstance(e, KeyboardInterrupt):
raise e
_shared_description_cache.clear()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how you found what was leaking memory?

And remind me of the flow please - is the cache built while parsing a save, then this line clears it after one save is parsed, to free that memory?

Copy link
Collaborator Author

@MichaelMakesGames MichaelMakesGames Dec 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious how you found what was leaking memory?

Nothing fancy. I'm remembered that was a relatively recent change, so I tried disabling the cache entirely, and that fixed the memory leak. Then I experimented to find a way to keep the cache (since that significantly increased speed) but not leak memory

The cache is gradually built up as the function is used, then cleared when we're done processing a save. I'll add a comment in the code here

@MichaelMakesGames MichaelMakesGames merged commit 95426dd into master Dec 15, 2024
3 checks passed
@MichaelMakesGames MichaelMakesGames deleted the parser-and-processor-fixes branch December 15, 2024 17:27
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.

2 participants