search

AppleWin / AppleWin

Apple II emulator for Windows
GNU General Public License v2.0
717 stars 166 forks source link

Save State Default Folder #691

Closed exoscoriae closed 4 years ago

exoscoriae commented 5 years ago

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 commented 5 years ago

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

exoscoriae commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

Seems as though we came up with similar work arounds.

tomcw commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

tomcw commented 4 years ago

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

tomcw commented 4 years ago

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

exoscoriae commented 4 years ago

thanks for the update. =)

tomcw commented 4 years ago

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

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>.

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

@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 commented 4 years ago

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

Python-Exoproject commented 4 years ago

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 commented 4 years ago

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

tomcw commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

Really what the 3 paths represent are:

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:

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 commented 4 years ago

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:

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:

I will try this, and then give an update.

sicklittlemonkey commented 4 years ago

What happens if both are specified?

I tried to give some thought to this after your second to last comment/email, but it's hard to sort through the use cases unless one is actually using it. ; - )

tomcw commented 4 years ago

What happens if both are specified?

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

Some other things:

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

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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):

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

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 commented 4 years ago

I agree it's nice to have a written discussion of thoughts and options to look back on. (Next time!)

The GUI behaviour sounds reasonable as it's sequential, so the user should get what they expect.

For the CLI we could just document that "last path used wins", and add an explicit switch to set the save path if someone wants it (processed last).

tomcw commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I keep closing the issue. oops

tomcw commented 4 years ago

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 commented 4 years ago

Tested and confirmed all working. Issue can be closed.

exoscoriae commented 4 years ago

ya, looks good on my end.