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

Save State Default Folder #691

Closed
exoscoriae opened this issue Sep 16, 2019 · 30 comments
Closed

Save State Default Folder #691

exoscoriae opened this issue Sep 16, 2019 · 30 comments
Labels
Milestone

Comments

@exoscoriae
Copy link

When using command line to launch applewin with a specific dsk\woz\etc file, pressing f11 to save state will default to the last folder you loaded an image from rather than the current one.

eg: if I run a command line to load choplifter, then exit, then run a command line to load championship lode runner, and then press f11, it will open the choplifter folder.

I am currently working around this by launching the current game twice. Once with "-screenshot-and-exit" and then again. By launching it twice, it then defaults to the last folder, which happens to be the same as the current folder.

@tomcw
Copy link
Contributor

tomcw commented Sep 16, 2019

Thanks for creating a new issue for this.
(Perhaps similar to #663.)

@exoscoriae
Copy link
Author

I may be misunderstanding #663 but my relative paths are working with the -d1 flag. But yes, further down there is talk of setting the current directory, so I imagine it would directly relate to that.

@tomcw
Copy link
Contributor

tomcw commented Jun 1, 2020

From #790, @Harbinger-365 said:

While looking into this I think I found whats causing #691 as well. I think its something to do with when the "starting directory" line is being updated when using the -d1-d2 command line switch. When you load applewin and press f11 or f12 it will open whatever folder was in the "starting directory" line before applewin loaded, not the folder of the game currently loaded. This makes me think that whatever is behind f11\f12 is looking at the "starting directory" line before it has been updated to the current path. I currently get around this by loading a game, quiting, and loading it again in my bat file. Its even worse when using the -h1-h2 command line switch as it never updates the "starting directory" line at all due to using "HDV Starting Directory" so even on a subsequent load f11\f12 will not load the folder of the current game. For some reason F3\F4 is not affected by this issue and will always open the current folder.

@exoscoriae
Copy link
Author

Seems as though we came up with similar work arounds.

tomcw added a commit that referenced this issue Jun 6, 2020
. support relative paths (#663)
. updated the current directory with the path for each loaded image (#663) & saving state (#691)
. added a new switch -current-dir <path>
@tomcw tomcw added the bug label Jun 6, 2020
@tomcw tomcw added this to the 1.29.14 milestone Jun 19, 2020
@tomcw
Copy link
Contributor

tomcw commented Aug 18, 2020

@exoscoriae - there's a new AppleWin 1.29.14.0 here, which contains an update for this issue. Can you give it a try?

@Python-Exoproject
Copy link

Python-Exoproject commented Aug 19, 2020

Hi @tomcw ,

I have tested the latest release for the save\load state issue I had. Results as follows:


Using Load State (F12): Seems to be no change on this one for my issue, when loading a disk image with -d1 and go straight to using F12 to load save state the shown folder is still that of the last -d1 image loaded before the current one. As before if current image is loaded with -d1 then exiting and loading again updates the behind the scenes value and it will be correct. For example:

Load Lode Runner using:

C:\AppleII\Images\LodeR> ..\..\AppleWin\Applewin.exe -conf ..\..\AppleWin\settings.ini -current-dir C:\AppleII\Images\LodeR -d1 "C:\AppleII\Images\LodeR\Championship Lode Runner (1984).dsk"

close AppleWin and then run:

C:\AppleII\Images\ChopL> ..\..\AppleWin\Applewin.exe -conf ..\..\AppleWin\settings.ini -current-dir C:\AppleII\Images\ChopL -d1 "C:\AppleII\Images\ChopL\Choplifter! (1982).dsk"

then press F12 to load a previous save state I will be presented with the folder C:\AppleII\Images\LodeR not the expected C:\AppleII\Images\ChopL. If I exit AppleWin and load Choplifter again I will be presented with the correct folder. I saw you mention a registry value being updated after a successful load\save "REGVALUE_SAVESTATE_FILENAME" which then updates "g_strSaveStatePath", could this issue be due to the fact that I am using an ini file for my settings and not the registry? Or if thats not the case would updating "g_strSaveStatePath" with current-dir folder from the command line switch at loading of the image instead of on a successful load\save be a fix?

As before using -h1 to load a hard drive image is in a slightly worse position as F12 is still using the -d1 disk image loaded as its pointer and this isnt being updated when loading a hard drive image, therefore even exiting AppleWin and loading the hard drive image again doesnt point F12 to the correct folder, it always shows the folder of the last -d1 image.


Using Save State (F11):

This is working pretty much as intended with the latest changes, so maybe the solution to the above Load State issue is contained in whatever you did to Save State? The only remaining item (and its a minor one) is again with using -h1. When you have loaded a hard drive image and press F11 to save state the folder shown is correct, however the default .yaml filename is that of the last -d1 disk image loaded.


Many thanks for your efforts on this issue!

@exoscoriae
Copy link
Author

@tomcw hello there. Just checking to see if there was anything we could do to help with this.

@tomcw
Copy link
Contributor

tomcw commented Sep 19, 2020

Thanks for the detailed repro example. I'll aim to take a look this coming week.

@tomcw tomcw modified the milestones: 1.29.14, 1.29.15 Oct 7, 2020
@tomcw
Copy link
Contributor

tomcw commented Oct 7, 2020

Not had time to check this yet. I've added it to the 1.29.15 milestone to ensure it gets considered.

@exoscoriae
Copy link
Author

thanks for the update. =)

@tomcw
Copy link
Contributor

tomcw commented Oct 14, 2020

In the Registry (or conf.ini), we have:

  • Preferences: 'Starting Directory' - for the path 'Last Disk Image 1' (and 2)
  • Preferences: 'HDV Starting Directory' - for the path 'Last Harddisk Image 1' (and 2)
  • Configuration: 'Save State Filename' - for the path and filename

Currently -current-dir <path> only takes affect after -d1,-d2,-h1,-h2 type commands, and doesn't impact the operation of load/save save-state, which as you can see (above) has it's own Registry entry ('Save State Filename').

I guess -current-dir <path> should also take precedence over the Registry entry ('Save State Filename'). The rationale being that the user wants to force a current folder, and for all operations to be based from there, not some previous save-state folder.

So F11(save state)/F12(load state) will then default to the "path" set by -current-dir <path>.

  • But if you quit AppleWin without load/saving a save-state then this "path" won't be updated in the Registry (or conf.ini)... so it will remain the old value.
  • Only if you successfully load/save a save-state, then 'Save State Filename' will get updated in the Registry (or conf.ini).

NB. This is consistent behaviour with using -current-dir <path> and not inserting/swapping a new floppy disk, ie:

  • Use -current-dir <path>
  • Quit AppleWin without inserting/swapping a new floppy disk
  • Preferences: 'Starting Directory' won't be updated in the Registry (or conf.ini)

@exoscoriae - how does all this sound?


The only remaining item (and its a minor one) is again with using -h1. When you have loaded a hard drive image and press F11 to save state the folder shown is correct, however the default .yaml filename is that of the last -d1 disk image loaded.

What happens if you have floppy in drive-1 and a harddisk as unit-1 - what should the default (save-state) .yaml be in that case?

@exoscoriae
Copy link
Author

This sounds great. @Python-Exoproject - how about you? And I think you can speak towards the question at the end best.

@Python-Exoproject
Copy link

Sounds like that will fit the bill, thanks again @tomcw for sorting this.

Regarding default save state name I think for our use case favouring the harddisk would be best. That way if just a harddisk is mounted it will be correct, and if both floppy and harddisk are mounted, then in our case they would most likely be for the same game anyway and therefore would still be ok.

However my concern with using the harddisk name as the default is that for the next game that only mounts a floppy would it use the name of the current mounted floppy or the name of the last harddisk image that was loaded? Would ensuring use of -s7-empty-on-exit for all harddisk games ensure that wont happen?

@exoscoriae exoscoriae reopened this Oct 15, 2020
@exoscoriae
Copy link
Author

didn't mean to close the issue. oops =)

@tomcw
Copy link
Contributor

tomcw commented Oct 15, 2020

Just a further thought about this:

Above I'm proposing that -current-dir <path> doesn't automatically change the Registry (or conf.ini) for the 3 path/pathnames for floppy, harddisk & save-state.

But if you wanted to force the change into the Registry (of conf.ini) then you'd need to manually insert a floppy image, plug-in a harddisk image & load/save a save-state file!

EG, this could be confusing for the user if they think it'll auto-update for all 3, but they only do one of these operations (eg. insert floppy). Then sometime later (without -current-dir) they attempt do a different operation (eg. F11/save state) and it reverts to some previous path for save-state files!

So I'm now thinking that -current-dir <path> should automatically update these 3 paths.

Additional: maybe add a new Preferences: 'Save-state Directory' for parity with the other 2 paths.

@Python-Exoproject
Copy link

You raise a good point, and your new solution sounds even better. I also like the idea of a Save-state Directory preference, would this be command line driven as well?

@tomcw
Copy link
Contributor

tomcw commented Oct 18, 2020

I've start the implementation, but hit more issues.

Really what the 3 paths represent are:

  • disk image path: where all my disk images are
  • harddisk image path: where all my harddisk images are
  • save-state path: either points to the disk or harddisk images path - so having a dedicated preference for 'Save-state Directory' is overkill really.

So I'm now thinking that -current-dir should automatically update these 3 paths.

One problem with blindly just updating all 3 paths is: what if I want to only update my disk image path, and leave my harddisk path alone?

I could have both a -current-disk-dir <path> and -current-harddisk-dir <path> (+ someway to specify the save-state path).

Or sticking with just -current-dir <path> the logic could be:

  • Update Registry (for [Disk] 'Starting Directory') if: you specify a disk image on the cmd line or there are no disks in any drives.
  • Update Registry (for 'HDV Starting Directory') if: you specify a harddisk image on the cmd line or there are no harddisks plugged in.

I don't think this is intuitive for the user... so perhaps the -current-disk-dir & -current-harddisk-dir is the better way to go here.

I will continue to think about it...

@tomcw
Copy link
Contributor

tomcw commented Oct 19, 2020

I'm going to explore the approach of having both -current-disk-dir <path> and -current-harddisk-dir <path>.
And the logic for implicitly setting the save-state path is:

  • if just the -current-disk-dir switch is specified then use that for the save-state path
  • if the -current-harddisk-dir switch is specified then use that for the save-state path
    • ie. just the -current-harddisk-dir switch, or in conjunction with the -current-disk-dir switch

At this stage I won't provide a -current-save-state-dir switch. I don't really see the need, and it's just another source of confusion (and an extra test path). And I will deprecate the -current-dir switch.

Are there any problems with this approach?
Well the save-state only saves the filename for disk & harddisk image files.
So this won't allow support for a save-state referencing both disks & harddisk image files, where the save-state is in the "disk image" directory, ie:

  • \mydisk.dsk
  • \mydisk.aws.yaml
  • \myharddisk.hdv

I will try this, and then give an update.

@sicklittlemonkey
Copy link
Contributor

sicklittlemonkey commented Oct 20, 2020 via email

@tomcw
Copy link
Contributor

tomcw commented Oct 20, 2020

What happens if both are specified?

I edited my comment (above yours) with a sub-bullet to clarify.

Some other things:

  • -current-disk-dir <path> or -current-harddisk-dir <path> can occur anywhere on the command line and have the same behaviour (ie. they can occur before or after image-load switches)
  • -current-disk-dir <path> or -current-harddisk-dir <path> take affect before any image-loading switches

So -d1 <image> -current-disk-dir <path> will use path to prefix images with relative paths.
EG:

  • -d1 mydisk.dsk -current-disk-dir C:\diskimages loads d1 with C:\diskimages\mydisk.dsk
  • -d1 ..\mydisk.dsk -current-disk-dir C:\diskimages loads d1 with C:\diskimages\..\mydisk.dsk
  • -d1 mydisk.dsk -current-disk-dir diskimages loads d1 with <cwd>\diskimages\mydisk.dsk (where cwd is the current directory)
  • -d1 C:\tmp\mydisk.dsk -current-disk-dir C:\diskimages loads d1 with C:\tmp\mydisk.dsk (so -current-disk-dir isn't used for this absolute path)

But if you don't specify -current-disk-dir <path>, then treat relative to the current directory... not the -current-disk-dir <path> from a previous launch of AppleWin (*). This means that these cases will continue to work.

Basically as you only have visibility of the command line (and not the Registry), then the behaviour should be intuitive based on what you have typed at the command line.


(*) Even though -current-disk-dir <path> will have persisted path to the Registry from the previous AppleWin run.

@exoscoriae
Copy link
Author

so... I understand this is a total over simplification of the issue, but it is still where all this comes from. So maybe restating it helps.

The simple goal here is... if I load Game A, I should be loading Save States for game A as well. That is really all there is to it. Prior to this, it would default to the folder from the last game... so lets call that Game B. Save States for game B are never useful when playing Game A.

So while I understand there are a lot of technical what-if's here... the simple question is, does it allow the user to load save states for the game they loaded via the command line? If yes? Then it works. If not... then we are back at square one.

Unless I am missing a scenario outside of my case use where a user would want to load Game A but access Game B save states, then I think simply defaulting to the same folder it loaded the game from solves the issue.

Granted, I haven't worked with this enough to understand why some games have floppy images, some have hard drive images, and some have both. Presumably... hard drive images mean the game required an install? Either way, for our use, the hard drive image and floppy image will be loaded from the same folder I believe. Is this right @Python-Exoproject ?

Unfortunately, I'm not feeling very useful in this conversation currently as it appears to have gone down a rabbit hole of what-ifs... and I think I must be looking at the problem too simply as it seems rather simple to me to load save states from whatever game is loaded at the time. Hopefully something in my above ramble helps in some way.

@tomcw
Copy link
Contributor

tomcw commented Oct 21, 2020

Hi @exoscoriae - I agree that I've been down a rabbit hole here! I too ultimately prefer a simple solution, so will take a step back to reconsider things.

FWIW, it's been useful to consider things in the wider context and documenting it (vs. mentally doing it, but not writing it down), but much of these nuanced cases are not being requested by anyone.

@tomcw
Copy link
Contributor

tomcw commented Oct 21, 2020

Ditching the proposal of -current-disk-dir / -current-harddisk-dir switches completely, and aiming for a simpler approach...

GUI (AppleWin's Configuration, or F11/F12 interface for save/loading save-states):

  • plug in a harddisk
    • if harddisk's path is different to current save-state's path then update the path, and drop the filename
    • otherwise (paths match) so no affect on save-state path
  • insert a disk
    • if there's a harddisk plugged in then no affect on save-state path
    • else if disk's path is different to current save-state's path then update the path, and drop the filename
    • otherwise (paths match) so no affect on save-state path

What happens if user inserts 2 disk images from different paths?

  • This can be a valid use-case, since user may be switching from game-A (in folder-A) to multi-disk game-B (in folder-B).
    • In this use-case, the save-state path will reflect the correct new folder-B path.
  • Equally there is the invalid case, where eg. user has DOS3.3 in Drive-1 (from folder-A), and a DOS3.3 data disk in Drive-2 (from folder-B).
    • AppleWin should raise an error if an attempt is made to save the state here.
    • NB. Currently, closing then restarting AppleWin will result in only the disk in one of the drives being re-inserted.

CUI (AppleWin's command line interface)

Similar to GUI.

What happens if user inserts 2 disk images from different paths?
Could just define this as an error, in which case a MsgBox is displayed, and the cmd line switch is ignored.

@sicklittlemonkey
Copy link
Contributor

sicklittlemonkey commented Oct 21, 2020 via email

@tomcw
Copy link
Contributor

tomcw commented Oct 23, 2020

OK, what I ended up implementing (in PR #849) is a slightly different (but simpler) thing to the idea immediately above. Now the save-state path & filename are derived at the point of saving/loading the state file.

The current logic is in the PR's initial comment.
I'll copy it here once I've done more testing and have committed it.

@exoscoriae
Copy link
Author

that sounds great to me. seems to make sense logically. I suppose there might be a few one off situations that act up, but those can always be worked around individually.

@exoscoriae
Copy link
Author

I keep closing the issue. oops

tomcw added a commit that referenced this issue Oct 25, 2020
…#691) (PR #849)

Whenever harddisks/disks are inserted (or removed) and *if path has changed* then:
. Then the internal save-state's path & filename will be updated to reflect the new defaults.
. LoadConfiguration(): Read the save-state pathname from Registry before harddisks/disks.

Also:
. CiderPress: only save pathname on OK.
. Refactored CPropertySheetHelper::BrowseToFile().
. Extended support for -d1,-d2,-h1, etc such that if the param is "", then it will eject/unplug the disk/harddisk.
@tomcw
Copy link
Contributor

tomcw commented Oct 25, 2020

There's a new release of AppleWin 1.29.15.0 containing this update: here. Once @exoscoriae or @Python-Exoproject confirm this version is OK, then I will close this issue.

@Python-Exoproject
Copy link

Tested and confirmed all working. Issue can be closed.

@exoscoriae
Copy link
Author

ya, looks good on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants