Skip to content

Not enough memory warning screen#96

Merged
contariaa merged 4 commits into
contariaa:mainfrom
me-nx:memory-warning-screen
Oct 3, 2025
Merged

Not enough memory warning screen#96
contariaa merged 4 commits into
contariaa:mainfrom
me-nx:memory-warning-screen

Conversation

@me-nx

@me-nx me-nx commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Follow-up on #65.

Added a warning screen shown on startup if JVM's max memory (-Xmx) is below a set threshold.

Crystal15118 and others added 2 commits September 11, 2025 10:24
Added a method which checks the ram allocation and then gives a warning if it is offset by 1100 MB from the recommended max ram allocation. The offset for 1100 is set because AA-like categories use about 1000 MB more than other categories. MultiMC's Ram Allocation amounts may pass the 1000 MB barrier, to avoid it we add 100 more to the 1000 MB. There's also a system property to disable it.
Replaced the log message with a screen, removed the upper memory limit.
Comment thread src/main/java/me/contaria/seedqueue/SeedQueue.java Outdated
@me-nx

me-nx commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

fyi @maskersss @Crystal15118

@contariaa

Copy link
Copy Markdown
Owner

This looks pretty good, I have a couple things I would like changed though:

  • I'd prefer if the warning screen was shown when actually starting a session rather than on startup, since thats when the player actually decides to play with the settings they configured
    • imagine a player starting up seedqueue for the first time, setting their settings, playing without warning, then the next day they come back and suddenly there is a warning screen that wasnt there before
    • this would probably require a small change in architecture, where seedqueue is actually started when pressing gold boots rather than after one reset. i've wanted to make this change anyway, it just depends on making antiresourcereload synchronized.
  • I think the warning screen should contain more information
    • in its current state players would probably only understand it if they already know about seedqueues ram requirements and how to change allocation, it should tell the user why they need to allocate more memory and that they could alternatively reduce their max queued seeds too (could even factor in how much memory is even available on the system, it doesnt really make sense to tell a player to allocate 10GB of ram if they only have 8GB available).
    • additionally a link to a new seedqueue wiki page could be considered

- Change the low memory warning to trigger when starting resetting,
not on launch, as it depends on the max queued seeds config.

- Mention max queued seeds in the warning, and link to the wiki.
@me-nx

me-nx commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

I'd prefer if the warning screen was shown when actually starting a session rather than on startup

Makes sense, I've moved the warning to starting resetting. Idk if this injection point is ideal though, but seems to work fine?

I think the warning screen should contain more information

I've added a mention of max queued seeds and a link to a non-existing wiki page, I have a stub here if you'd like to add it (although you'd have to update image links).

@me-nx

me-nx commented Sep 12, 2025

Copy link
Copy Markdown
Contributor Author

could even factor in how much memory is even available on the system, it doesnt really make sense to tell a player to allocate 10GB of ram if they only have 8GB available

What should be the threshold if I were to change that? It probably doesn't make sense to recommend allocating 6 gb if they have 8 gb total, but idk that depends on a lot of factors.

@contariaa

Copy link
Copy Markdown
Owner

What should be the threshold if I were to change that? It probably doesn't make sense to recommend allocating 6 gb if they have 8 gb total, but idk that depends on a lot of factors.

I know maskers has some threshold calculation in pingus recommended_settings, probably best to just copy that, I'll ping him on discord

@contariaa contariaa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This will currently also show a warning for people who havent actually set up seedqueue but have a very low amount of allocated ram (iirc multimc defaults to 1GB). While this isnt inherently bad, the warning screen doesnt accurately tell the user whats going on, so it would probably be easiest to just disable the check if config.maxCapacity is 0.

I'm also a bit worried that the link may not be super obvious, not everyone will hover the text since links arent very common in Minecrafts UI, I'd prefer just adding the link to the end.

Comment thread src/main/java/me/contaria/seedqueue/SeedQueueConfig.java Outdated
Comment thread src/main/java/me/contaria/seedqueue/gui/MemoryWarningScreen.java Outdated
Comment thread src/main/resources/assets/seedqueue/lang/en_us.json Outdated
@me-nx me-nx requested a review from contariaa September 21, 2025 00:23

@contariaa contariaa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good to me, thank you for the PR :)

@contariaa contariaa merged commit f3c8db7 into contariaa:main Oct 3, 2025
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.

3 participants