Feature/1.20.x/UI Overhaul

[Gerbagel]

Changes proposed in this pull request:

Complete UI overhaul:

• new menu registry system for submods
• every submod that needs a menu has one
• new HUD
• limit on ingested metal

Testing checklist:

• yes I tested this in a runServer environment, and therefore won't make leaf cry
• yes, I actually connected to that server and verified it worked and don't need to have my kneecaps removed
• yes, I also tested the built jars manually outside of the development environment, because problems sometimes don't show up inside the dev environment.

I will test this later :)

[claude]

Code Review — Feature/1.20.x/UI Overhaul

Overall the new registry-based menu system is a clean concept, but there are several issues that need to be resolved before this merges. Inline comments mark individual lines; this comment covers the big-picture findings.

---

Critical — Will crash or silently corrupt

1. SpiritwebHud NPE on every HUD render when no power is selected (see inline comments on lines 65 and 172)

getSelectedManifestation() returns null when the player has no power selected. renderUsage() dereferences the result at line 65 unconditionally; renderText() does the same at line 172. renderIcon() already guards with if (selectedManifestation != null) — the other two render methods need the same guard. This will crash the client on every HUD frame for any player with no active power.

2. handleScroll NPE in ClientForgeEvents (line 45–46, pre-existing but unguarded)

player.getItemInHand() is called immediately after Minecraft.getInstance().player with no null check. onKey (line 68) correctly guards, but handleScroll does not. Scrolling the mouse wheel in the main menu or at world load will crash the client.

3. FeruchemyChargeThread — background thread reading unsynchronised game state (see inline comment on line 102)

The background thread accesses mc.player, mc.level, and iterates mc.player.getInventory().items without any lock. The client game thread modifies all of these between ticks. This is a live data race: at minimum a ConcurrentModificationException during inventory iteration; at worst silent data corruption or a hard crash during respawn/dimension change.

4. Mixin refmap DSL entries all commented out (see inline comment on build.gradle line 481)

All add sourceSets.* calls in the mixin {} block are disabled. Without them the refmaps are not wired into the jar by MixinGradle's standard mechanism. The manual -AoutRefMapFile workaround may generate the files during compilation but will not package them correctly in production builds. Any mixin that targets a vanilla method will fail to remap in production jars and crash on startup.

---

Major — Correctness bugs

5. Non-volatile isStopping / t in FeruchemyChargeThread (line 29)

Both fields are written on the game thread and read on the background thread. Without volatile the JVM is allowed to cache the old value; the background thread may spin forever after stop() is called.

6. tryLock() silently drops inventory updates (line 205)

If tryLock() returns false, the entire update to feruchemyChargeMap / feruchemyMaxChargeMap is silently skipped. Use blocking lock() for the write since the critical section is tiny.

7. Unchecked cast from getSubmodule() in new AllomancyManifestation methods (lines 265 and 272)

getSubmodule(ALLOMANCY) can return null for non-Mistborn entities. Both new getInvestitureHud and getInvestitureRemaining methods cast unconditionally; use AllomancySpiritwebSubmodule.getSubmodule(spiritweb) and null-check the result. The same double-cast pattern at line 119 (((SpiritwebCapability) data)) is pre-existing and also needs attention.

8. Static mutable fields in SpiritwebMenu (lines 32–33)

selectedManifestationType and selectedManifestation are static, so they persist when the screen is reopened. init() picks up stale values from the previous session, potentially initialising the wrong tab. These should be instance fields initialised from spiritweb.getSelectedManifestation().

9. FeruchemyClientSetup.renderNicrosilHUD unchecked cast (line 61, pre-existing)

(SpiritwebCapability) cap — cap is ISpiritweb. If a different implementation is ever used this throws ClassCastException. Use the interface methods directly.

10. FillMetalReservesCommand: no null check on getSubmodule() result (line 46, pre-existing)

AllomancySpiritwebSubmodule.getSubmodule(spiritweb) can return null. asm is used without a guard at line 57.

---

Minor

• Dead code — FeruchemyChargeThread lines 198–203: CuriosApi.getCuriosInventory(mc.player).map(...) result is immediately discarded. (see inline)
• Raw GL11 blend state — AllomancySpiritwebMenu.renderBorder() sets a custom blend function with GL11.blendFuncSeparate and never restores the default. Use RenderSystem.defaultBlendFunc() and remove the redundant setShaderTexture call. (see inline)
• Double capability lookup — ClientForgeEvents.onKey at line 77 calls SpiritwebCapability.get(player) again inside an already-resolved ifPresent lambda. Use the already-resolved spiritweb reference. (see inline)

---

The build.gradle mixin change and the SpiritwebHud NPEs are the most urgent — both will affect every player on every server tick once deployed.

[Gerbagel]

Get cancelled, Claude. Anyway, I think it's good to merge now.

[Gerbagel]

Get cancelled, Claude. Anyway, I think it's good to merge now.

Nevermind, just realised I forgot to implement Atium buttons

[Gerbagel]

There, added atium buttons :)