feature(mods): Mod Manager UI, Rescan and Hot Reload reflect deleted mods#236
Open
Zorkats wants to merge 4 commits into
Open
feature(mods): Mod Manager UI, Rescan and Hot Reload reflect deleted mods#236Zorkats wants to merge 4 commits into
Zorkats wants to merge 4 commits into
Conversation
Phase 1 of the in-game Mod Manager (docs/mod_manager_menu_plan.md): turn the Mods panel's single Hot Reload button into a read-only list of installed mods with their manifest metadata and load state. New port/mods/ModRegistry — the model behind the panel. A pure, no-throw Snapshot() walks the engine's mounted archives (ArchiveManager), treats any archive carrying a manifest.json as a mod (same rule MountModsDir uses, so the base game + shader archives are excluded), reads name/version/author/ description from Archive::GetManifest(), and marks each Loaded when its manifest name is in ScriptLoader::GetLoadersInDependencyOrder(). Results are ordered loaded-first then case-insensitive by name for a stable list. PortMenu: the Mods panel now renders an action bar (Hot Reload / Rescan / Open Mods Folder), a name/author filter, status-badged mod cards (LOADED / not loaded / manifest issue) with an archive-path tooltip, and a footer count. The list is cached and refreshed on first view, Rescan, and after a Hot Reload rather than re-scanned every frame. No enable/disable yet (Phase 2). Additive and behavior-preserving: with no mods the panel just reports an empty folder. Model/view are separated so discovery stays UI-independent. Verification: ModRegistry compiles clean under clang -std=c++20 -Wall -Wextra against the libultraship API, and its logic was exercised by an edge-case harness (base-archive exclusion, null subsystems, unnamed manifest fallback, loaded matching, case-insensitive ordering). A full in-tree build was not run here (submodules not checked out in this checkout); the libultraship calls are verified against pinned SHA be6415584d.
PortShutdown relied on ~Context (via sContext.reset()) to run
ScriptLoader::UnloadAll, but by then the Context's EventSystem is already
being destroyed. A mod that REGISTER_LISTENERs in ModInit and
UNREGISTER_LISTENERs in ModExit then crashes on exit:
UnloadAll -> ModExit -> UNREGISTER_LISTENER -> Context::GetEventSystem
dereferences a dead shared_ptr<EventSystem> (access violation in
_Ptr_base<EventSystem>::_Incref).
Unload the scripts explicitly in PortShutdown, while the EventSystem is still
alive, right after the hooks are torn down. The later ~Context UnloadAll then
finds nothing loaded.
Verified: graceful close (WM_CLOSE) with a listener-registering mod loaded now
logs the mod's ModExit ("[PlayerTint] exit OK") and exits with no crash dump,
where before it produced one on every exit.
MountModsDir only ever adds archives, so a mod folder/.o2r deleted at runtime stayed mounted in the ArchiveManager for the session: Hot Reload recompiled it from the still-mounted VFS (still LOADED) and Rescan still listed it. Add UnmountMissingMods() (port.cpp): for each mounted archive carrying a manifest.json whose on-disk path no longer exists, ArchiveManager::RemoveArchive. The base game + shader archives (no manifest) are untouched. Wire it into Hot Reload (after UnloadAll, before re-mount/recompile) and Rescan (sync mounts to disk, then snapshot). Verified in-game: deleting a loaded mod's folder and pressing Rescan/Hot Reload now drops it from the list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Currently, Battleship does have a menu for Hi-Res Texture Packs and more,
but not a proper Mod menu where people can see what Mods they have installed,
or even reload or scan for new mods. This fixes that.
One issue that I had during development of this mod menu was that
the Mods menu's Rescan and Hot Reload buttons did not pick up a mod
folder deleted at runtime — the mod kept showing
LOADEDand Rescan stilllisted it.
Why
MountModsDir(port/port.cpp) only ever adds archives (skipsalready-mounted,
AddArchives new). Nothing unmounts. So a deleted mod folderstays mounted in the
ArchiveManagerfor the session:archive →
LOADEDagain.Fix
Add
UnmountMissingMods()(port/port.cpp): for each mounted archive carrying amanifest.jsonwhose on-disk path no longer exists, callArchiveManager::RemoveArchive. The base game + shader archives (no manifest)are never touched.
Wire it in (
port/gui/PortMenu.cpp):DoHotReload): afterUnloadAll, before re-mount/recompile —so a deleted mod is dropped instead of recompiled.
RefreshModList): sync mounts to disk (drop gone, pick up new),then re-snapshot. New mods appear as not loaded until a Hot Reload compiles
them; this only refreshes the inventory.
Test plan
Load with a mod, delete its folder while running, press Rescan (then Hot
Reload). It should drop from the list. Drop a new folder in and Rescan — it
should appear (not loaded), then load on Hot Reload.
Notes
itself needs a button click to exercise, hence the manual test plan.
ArchiveManager::RemoveArchive, the unmount primitive flagged in the ModManager design note as the main Phase-2 dependency.