FelicitusNeko / meritous-ap

Lancer-X's procedurally-generated dungeon crawler, modified for Archipelago Multiworld
https://archipelago.gg/tutorial/Meritous/setup/en
GNU General Public License v3.0
0 stars 2 forks source link

add basic connection menu #23

Open Rnd-Guy opened 2 months ago

Rnd-Guy commented 2 months ago

PR changed - Add connection menu

Adding connection menu and removing need for meritous-ap.json.

Due to problems with both normal gamesave handling and simple slotname specific savefile handling when room counts are added, I've changed the goal of this PR to add a basic menu to add a basic connection menu.

Problems with normal saves with room counts added:

Problems with the original PR implementation:

365463917-7f8564d5-e93c-4737-aefa-da536036c16b

Old PR text: add slotname specific savefiles

~~Changes the save filename depending on the slotname in meritous-ap.json to SaveAP_slotname.sav Have tested it with Player1 and Player2 and it switched fine (though meritous needs to be closed and reopened to reread the json) When ap-enabled is false, the savename is just SaveAP.sav~~

Unfortunately meritous-ap.json is only read on load so there might be a slight issue if people try to edit the json without restarting the game, maybe not a big issue though.

Might need to double check what characters work and what characters do not.

There's a fair bit of C specific code here that looks a bit messy (0 and 1 for boolean values, string concatenation being difficult) that I'm not sure what better way to use

Old gif: meritous-save-files

FelicitusNeko commented 2 months ago

I think overall, this would require a rework for how Meritous connects to the multiworld. Reason being:

FelicitusNeko commented 2 months ago

Oh, I see #24 changes how connecting works. Checked this one first 😅

Rnd-Guy commented 2 months ago

Extremely fair points! By itself yeah you're perfectly right this sorta introduces as many issues as it solves and as you say, an in game menu would be far superior!

What drove this was more a worry with the room count change:

Now that I think about it though, I was rather focused on not breaking savefiles but changing save file names does basically a similar thing haha.

Either way I think you're right this needs a bit of a rethink

Rnd-Guy commented 2 months ago

Ultimately I'd say the intention of this change is more to support the room count change, as opposed to being a fully fledged change on its own, which means it kinda should be together in 1 PR rather than this being separate.

Feel free to suggest what savefile changes you think might be needed for room counts to be implemented smoothly, and we can decide if we should combine the PRs or just close this one entirely or flesh this out some more!

Rnd-Guy commented 2 months ago

Decided to give making a menu a go:

(5fps to reduce gif size) meritous-new-menu

Only got it to type details in so far, it doesn't actually use them for anything and still reads the ap.json file.

Current structure: 1) APENABLED - archipelago or local randomiser a) if archipelago, go to CONNECTION, otherwise go to GAMEMODE 2) CONNECTION - fill in connection details. these are currently just strings. a) connect will then go to GAMEMODE if connection succeeds (needs something to say if it's connecting) b) cancel goes back to (1) APENABLED 3) GAMEMODE - new game, or continue game if file is found. Note if we got here by picking local randomiser, pressing escape will take us back to APENABLED, otherwise we'll go back to CONNECTION.

For this to work, the following still needs to be done:

Thoughts:

Will commit the code I have for now but this is definitely a work in progress and is very messy.