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

Script API: in FillSaveGameList() accept a range of slots #2289

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented Jan 9, 2024

I don't like adding "last minute" things to an upcoming release, but this request seemed easy enough to try.

Historically FillSaveGameList has a hardcoded range of 0 to 99 slots, and for some reason also additionally limits total shown slots to 50 items. This change lets user to tell which range of slots to fill, and removes item count limit, because it makes no sense anymore.

/// Fills the list box with the current user's saved games in the given range of slots.
import int  FillSaveGameList(int min_slot = 0, int max_slot = 99);

Backwards compatibility
Old compiled games will be using same old range (0-99) and 50 items limit.

@ivan-mogilko ivan-mogilko added type: enhancement a suggestion or necessity to have something improved context: script api labels Jan 9, 2024
... because what if the game only uses autosaves?
@ericoporto
Copy link
Member

This is related to #816 (I think)

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 9, 2024

It is related, but the #816 was about solving this when running already existing games without rescripting them.

This pr only lets game authors to show more in their games. There may have to be a separate change to let override this limit in existing games.

From my comment in that issue:

This leads to two questions:

  • how to deal with this in new games? Apparently script API has to be adjusted to either allow to limit number of saves filled with the call to ListBox.FillSaveGameList, or changed further and separate "regular" slots and special slots which do not mix. Or maybe save slot system may be revamped into something different in the future versions (but that's an issue for another day).
  • how to deal with this when running old games, if you want engine to override their saves limit. What I suggested earlier is a config option that would tell engine to just ignore this constraint. Of course in particular games this may lead to unconventionally used slots also getting in the list, but I don't think this can be helped, player should just be aware and careful.

I might actually add the config option which hacks this number for testing / running old games, which will likely be enough to close #816.

@ericoporto
Copy link
Member

I might actually add the config option which hacks this number for testing / running old games, which will likely be enough to close

Not sure if we need another hack for changing old things, if it's working as is, it's fine, better to look into the future - less options are less surface for bugs.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 9, 2024

I am speaking of running old games with the backwards compatible engine.

Ags 4 does not have to have this.

@ivan-mogilko ivan-mogilko marked this pull request as draft January 10, 2024 09:28
@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 10, 2024

I am actually going to suspend this, so converted to draft for the time being.

While the change to FillSaveGameList itself is really trivial, the consequence is that people may use any arbitrary range of saves.
And then this old DeleteSaveSlot behavior gets in a way:
https://adventuregamestudio.github.io/ags-manual/Globalfunctions_General.html#deletesaveslot

for historical reasons, when deleting slots in the range of 1 to 50, this function also moves highest slot found in that range to fill the freed slot. For example, if there were slots 1, 2, 3, 4, 5 and DeleteSaveSlot removes slot 3, then save in slot 5 will be renamed to become slot 3.

See commit: 8e07fcd

If I allow to list any arbitrary range of saves in the listbox, then this behavior must also be either adjusted or removed.
As a consequence, there's a chance this will break some game logic, because currently games might account for this, or even use this behavior on purpose.
Which in turn may lead to users requesting additional setup or commands (like RenameSaveSlot, for a hypothetical example, if they want to simulate this historical behavior).
In other words, this may require some extra api design, which should not be done in a rush.

EDIT: frankly, reading DeleteSaveSlot description again, makes me doubt if it's much useful at all, since it does not shift all the slots down, but only takes 1 topmost one and moves it to the gap. And that does not guarantee filling empty slots appearing after manual save file deletion.
But I still do not want to risk this now, because one change will likely bring more dependent changes and user requests.

*I know that saves may also be manipulated using a File api, and probably some do that; but I am not certain if that's a good thing, it's more of a hack, since "save slot" is a special item in script api, having their own dedicated functions. So maybe it's not wise to rely on raw File commands.

@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented Jan 10, 2024

I might remake this PR for ags4 instead, and later see if that is safe to backport or anything for some tiny 3.6 update.

EDIT: ags4 pr: #2293

@ivan-mogilko
Copy link
Contributor Author

Implemented in ags4 by #2293. This may be later backported to ags3 branch if really wanted.

@ivan-mogilko ivan-mogilko deleted the 361--fillsavegamelist-range branch January 20, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: script api type: enhancement a suggestion or necessity to have something improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants