Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New Feature] Load save states from command-line or playlist #13354

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

asciibeats
Copy link
Contributor

From my post to "Issues":

I added a feature to be able to autoload save states via cli or playlist. I call them entry states. My motivation is to get quickly into a preset game when playing with friends. For example: It takes AT LEAST two and a half minutes to get a 4-player game of "Winter Games" for the C64 going. It loads slowly and you have to enter your names every time. With entry states i can prepare those games in advance and then get into them instantly. I have now created a few fun playlists that contain small single-player challenges like "get to the end of the level" having x lives, short duels (directly load into a penalty shootout of a soccer game), the aforementioned sports-game descipline selection and a lot more. For me it's a game changer ;)

From command-line:

$ retroarch -L <core> -e /path/to/save.state <rom>

From playlist:

{
   "path": "<rom>",
   "state": "/path/to/save.state",
   ...
   "core_path": "<core>",
   ...
} 

I still don't really grasp what i would have to do to make it work for every use case and some guidance would be much appreciated, if you are interested in this feature that is.

Cheers!

@jdgleaver
Copy link
Contributor

Thanks, this is an interesting feature! I have a few small comments - I'll make these directly in the code.

command.c Outdated Show resolved Hide resolved
command.c Outdated Show resolved Hide resolved
command.c Outdated Show resolved Hide resolved
intl/msg_hash_us.h Outdated Show resolved Hide resolved
manual_content_scan.c Outdated Show resolved Hide resolved
menu/cbs/menu_cbs_ok.c Outdated Show resolved Hide resolved
menu/cbs/menu_cbs_ok.c Outdated Show resolved Hide resolved
playlist.c Outdated Show resolved Hide resolved
playlist.c Outdated Show resolved Hide resolved
playlist.c Outdated Show resolved Hide resolved
playlist.h Outdated Show resolved Hide resolved
retroarch.c Outdated Show resolved Hide resolved
tasks/task_content.c Outdated Show resolved Hide resolved
@jdgleaver
Copy link
Contributor

While I still think that every instance of entry_slot should be entry_state_slot, this seems okay now. It's a little inconsistent that regular states can have a valid slot index of 0 while entry states have to start at 1, but never mind - twinaphex thought it was okay to merge, so that's fine.

I'm sure you know about -Wdeclaration-after-statement -Werror=declaration-after-statement. Why not add those to the Makefile?

For info, C89 requires variable declarations to go at the top of the scope, no compiler flags can change that.

@asciibeats
Copy link
Contributor Author

asciibeats commented Dec 14, 2021

While I still think that every instance of entry_slot should be entry_state_slot, this seems okay now

I'm not too attached to it. So if you think it's not too late, change it. I think it is a good idea to be snappy towards the user. Maybe just keep the json field and cli option as "entry_slot" and "--entryslot" respectivly and change it in playlist_entry to "entry_state_slot"? Do as you like. I want you to be happy :) Seriously.

It's a little inconsistent that regular states can have a valid slot index of 0 while entry states have to start at 1, but never mind

Save state 0 seems to be a special case. It's named .state instead of .state0 and the first state created via frontend (xmb) is .state1. I did not dig too deep yet. Could you give me a hint why that is? Anyways, i like the default value to be the off-value and it's easy to handle.

For info, C89 requires variable declarations to go at the top of the scope, no compiler flags can change that.

I thought the warnings/errors would at least help exactly with that. Obviously i will try my best not to make that mistake again. Also memory leaks ;)

BTW next up for me is entry state management and frontend itegration. If i have any questions, may i reach out to you directly somehow?

@jdgleaver
Copy link
Contributor

I'm not too attached to it. So if you think it's not too late, change it. I think it is a good idea to be snappy towards the user. Maybe just keep the json field and cli option as "entry_slot" and "--entryslot" respectivly and change it in playlist_entry to "entry_state_slot"? Do as you like. I want you to be happy :) Seriously.

Don't worry - This issue becomes the responsibility of the one who merged the PR, not you or me ;)

Save state 0 seems to be a special case. It's named .state instead of .state0 and the first state created via frontend (xmb) is .state1. I did not dig too deep yet. Could you give me a hint why that is? Anyways, i like the default value to be the off-value and it's easy to handle.

I was referring to the fact that the regular save state slot index starts at 0 (in code and in the menu) - whereas any user interface for entry states will need to start at 1.

The naming of 'zero index' states without a number suffix is a legacy thing - I guess whoever first implemented that (a long time ago!) thought it would be 'cleaner' for users who generally leave the index at 0. And now it can't be changed, or we lose backwards compatibility.

I agree with you, however, that 0 = disabled makes handling the entry states much easier :)

BTW next up for me is entry state management and frontend itegration. If i have any questions, may i reach out to you directly somehow?

I'm afraid I don't always have time to guide people through difficult tasks, but if you want to post a draft PR at some point I will try to help. Also, it's a good idea to try posting on the libretro discord (in #programming or #programming-frontend) if you have specific questions.

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.

None yet

3 participants