Skip to content

add basic connection menu#23

Draft
Rnd-Guy wants to merge 2 commits intoFelicitusNeko:apclientfrom
Rnd-Guy:slot-specific-savefile
Draft

add basic connection menu#23
Rnd-Guy wants to merge 2 commits intoFelicitusNeko:apclientfrom
Rnd-Guy:slot-specific-savefile

Conversation

@Rnd-Guy
Copy link
Copy Markdown

@Rnd-Guy Rnd-Guy commented Sep 6, 2024

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:

  • Smaller rooms make having multiple saves significantly less straining, and as such it's a lot easier to pick up multiple saves. However loading the wrong save causes things to be calculated incorrectly, eg enemy scaling, crystal gain etc. so making it easier to handle multiple saves should help prevent this.

Problems with the original PR implementation:

  • If a player stops a seed, changes meritous-ap.json and then tries to load game without reopening the game, then they will find themselves still in the same seed. If they then do new game, they may end up overwriting the hard work they did on seed 1 despite ap.json pointing to seed 2. Though this was always possible, it is expected behaviour when it's just one savefile whereas it is unexpected when multiple is supposed to be supported.
  • Multiple seeds but sharing the same name will still overwrite each other.

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
Copy link
Copy Markdown
Owner

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

  • Slot name can easily be the same between MWs, which could lead to the same kind of save file conflicts that could already happen with just the one file
  • The better solution would be to derive the save file name from both the MW seed and slot name, which would also require connecting to the MW earlier
  • I've been wanting to implement a programmable menu to allow configuring MW connection from within the game without futzing with external JSON files

@FelicitusNeko
Copy link
Copy Markdown
Owner

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

@Rnd-Guy
Copy link
Copy Markdown
Author

Rnd-Guy commented Sep 6, 2024

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:

  • If I load a savefile that only has 100 rooms, but the server tells me I have 200, then the server's 200 will be set as the truth but the dungeon will not generate extra rooms, so there will only ever be 100 rooms, making the agate knife not spawn and my % explored is reduced causing reduced crystals and chests.

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
Copy link
Copy Markdown
Author

Rnd-Guy commented Sep 6, 2024

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 Rnd-Guy marked this pull request as draft September 8, 2024 19:28
@Rnd-Guy
Copy link
Copy Markdown
Author

Rnd-Guy commented Sep 8, 2024

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:

  • Read the details from here instead of ap.json
  • Create something to say when it is connecting, and if details are incorrect
  • Get seed from multiworld to add to save file name (progress: got seed, need to test)
  • Identify if the save file for a specific connection exists, and be able to load into said file
  • Be able to create a new game and have that create a new save file
  • Clean up the text so it's not being covered up by the title screen logo
  • Remove AP_ENABLED text
  • Test with jargon characters

Thoughts:

  • Couldn't think of a simpler structure without losing the ability to do training mode
  • A lot of games start the game straight away once you connect. Perhaps new game + training menu should only show if no save is detected, and load instantly otherwise. Can always delete the save from in folder after all.
  • Though getting the multiworld seed can help saves be truly unique, it might result in too long filenames. Will need to test this with max length room names
  • This will conflict with the room count PR due to order of operations, but hopefully it won't be too bad
  • Future PR idea: save the last connection config into a file and prepopulate it
  • Future PR idea: fully fledged loading screen that shows all current savefiles, and can prepopulate the connection details with the settings for that save file. Probably a bit too much work

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

@Rnd-Guy Rnd-Guy changed the title add slotname specific savefiles add basic connection menu Sep 8, 2024
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