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

DC: Use regular ScummVM menu #1375

Closed
wants to merge 4 commits into from
Closed

DC: Use regular ScummVM menu #1375

wants to merge 4 commits into from

Conversation

@tsowell
Copy link
Contributor

@tsowell tsowell commented Nov 3, 2018

This replicates the functionality of the Dreamcast game menu using regular ScummVM GUI widgets. Dialog widgets replace the plugin loading progress messages, LauncherDialog replaces the game menu, and MassAddDialog scans the CD for games.

To support this, MassAddDialog now has a (default false) "autoClose" flag which indicates that it should close immediately upon completion instead of waiting for user interaction.

The UI experience is similar to the current game menu:

  • Upon booting, plugins are loaded from the CD drive root. If there are no plugins in the root directory, a BrowserDialog prompts to choose a different plugin directory. During plugin loading, Dialogs display the plugin filename and the amount of memory available.

  • Before the launcher is displayed, a MassAddDialog scans the CD for games and automatically closes and adds them to the game list.

  • The game list updates when the lid is opened (to disable the games on the CD). When the lid closes, a new MassAddDialog scans the CD for games and updates the game list with any new games.

The main impetus for this is the SD card support I'm working on contributing - a lot of the benefits of the full ScummVM menu are unlocked with a writable config file. There are still advantages for CD-only use, including:

  • Consistency with other platforms

  • The ability to change options, even if they aren't saved for the next session

  • It fixes bug #7704 (DC: Add game chooser when more than one game detected)

That being said, I'm new here, and I know this is a big change and might not be the direction you want to take the Dreamcast port. Let me know if there's a better way to do any of this or if I should take a different approach.

@digitall digitall requested a review from zeldin Nov 3, 2018
@digitall
Copy link
Member

@digitall digitall commented Nov 3, 2018

@tsowell : Thanks for your contribution. This looks very interesting and as you indicate, it fixes a few bugs and makes the DC port more consistent with other ports. The issue with the DC is always the limited RAM available ie. 16Mb, so I think previously it has been a limited GUI to avoid too much overhead, though that may not be a "real" issue.

Can I ask if you have played any games with this build? And if so, which ones and there was sufficient RAM to finish them! :)

I have triggered a request for review by @zeldin who is our original DC porter, but overall I think this looks like a good change... assuming everything still runs without OOM :)

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Nov 3, 2018

There are a couple of issues to consider here.

The first is the memory situation. The issue here is not so much that the default GUI is taking a lot of memory, but rather that this change removes the plugin directory chooser, which is required for a distribution on a single disc to be possible. If you put all the plugins on a disc and scummvm loads all of them, you will run out of memory from just loading the plugins. So you can't even choose a game, much less start it. Thus I added the feature that you can partition the plugins into groups, and select a group when booting and only those plugins would be loaded. Previously I solved this by authoring multiple CDs with the different plugin sets, but the situation was becoming unwieldy.

The second is the lack of persistent storage. At least historically (I haven't checked in a while) the regular GUI worked by having a separate game scan function which saved the results for later use. something which does not work well on the DC (while you could save to a memory card, the same memory card might not be available on the next run), and also the concept of a "library" of available games does not map so well to a situation when you swap game discs in and out.

So while I haven't tested the patch yet. I feel that it is important to make sure that the following points are preserved:

  • It is possible to author a single CD with all the plugins on it and use it to play games on other disks, with only a single disk swap
  • You do not have to perform extra UI operations to add the game to your library every time scummvm is booted
  • Swapping game disks works well, and does not confuse the user as to which games can currently be launched

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Nov 3, 2018

Oh, one more thing:

  • The custom save icons for games that support them (e.g. Sam & Max) should still work

Loading

@digitall
Copy link
Member

@digitall digitall commented Nov 3, 2018

@zeldin: Thanks for those comments and background which should be useful to @tsowell. Hopefully you can get time to review / test this over the next few weeks, or maybe over Xmas break... I suspect I will end up working on bits then and avoiding awful TV :)

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Nov 4, 2018

@digitall: I don't think this should have an affect on the memory available to games, as the launcher gets popped off the stack before a game is launched, but I'll do some testing to measure the actual impact. I've only played through qfg1vga using the launcher (but not this build exactly).

@zeldin: Thanks for taking a look and for the feedback. I tried to make this behave as close as possible to the current menu, and I believe most of these have been addressed.

It is possible to author a single CD with all the plugins on it and use it to play games on other disks, with only a single disk swap

Yes, this works very similar to the current Dreamcast menu. If no plugins are found in the CD's root directory, it displays a BrowserDialog prompting to select a plugin directory. If the user chooses a directory, plugins are loaded from only that directory. From there, they can load a new CD and play games from it.

You do not have to perform extra UI operations to add the game to your library every time scummvm is booted

No user intervention is required. When ScummVM boots, a MassAddDialog pops up automatically and scans for games. When it has finished scanning, it automatically closes and adds all the games to the library.

Swapping game disks works well, and does not confuse the user as to which games can currently be launched

I think swapping disks works well. When the user opens the CD lid, any library games on the current disk are disabled and turn grey. When the user closes the CD lid, the new CD is scanned, and the games are either re-enabled or added to the menu.

The custom save icons for games that support them (e.g. Sam & Max) should still work

I had not tested the save icons, and they were not working. I've added a new commit that finds and loads a game's icon before starting the game, and they work now.

By the way, thanks for all your Dreamcast work over the years. I've been using your web site as a reference for about 15 years now, and it has been enormously helpful.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Nov 4, 2018

@tsowell: It sounds like you've done a solid job. 💯 I'll try to take a closer look and fire it up myself next weekend (this one was already fully booked unfortunately).

Loading

zeldin
zeldin approved these changes Dec 2, 2018
Copy link
Contributor

@zeldin zeldin left a comment

Hi. I finally got around to trying out these changes. They seem to work great! One thing I'd like to see though is for the plugin dir selector to start in the directory /PLUGINS (if it exists) rather than /. That would make the nero images more convenient to use.

I tried to make a savegame in Sam & Max Hit the Road (PC CD-ROM) to check that the icon scraping was working correctly, but for some reason it gave me "Not enough space available!" even with a completely empty card! I need to check this more thoroughly, it's probably completely unrelated to this PR...

Loading

if (!gameid.empty()) {
reportf("path %s\n", ConfMan.get("path").c_str());
Common::String path = ConfMan.get("path");
sprintf(icofn, "%s%s.ICO", path.c_str(), gameid.c_str());
Copy link
Contributor

@zeldin zeldin Dec 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use snprintf here.

Suggested change
sprintf(icofn, "%s%s.ICO", path.c_str(), gameid.c_str());
snprintf(icofn, sizeof(icofn), "%s%s.ICO", path.c_str(), gameid.c_str());

Loading

// Set the icon if a game has been selected
Common::String gameid = ConfMan.get("gameid");
if (!gameid.empty()) {
reportf("path %s\n", ConfMan.get("path").c_str());
Copy link
Contributor

@zeldin zeldin Dec 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of reportf should be guarded with #ifndef NOSERIAL

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Dec 2, 2018

Thanks for the feedback, @zeldin.

I think there are a couple of options for setting the plugin dir selector starting path. The BrowserDialog widget uses the browser_lastpath configuration key for the starting path, so one way would be to include in the Nero image's filesystem a scummvm.ini file with browser_lastpath set to "/PLUGINS". Or we could modify BrowserDialog so that the caller can optionally provide a starting path - I'll start looking into this approach.

I have run into "Not enough space available!" too, and the solution for me was to make sure that ScummVM was building with zlib support. Apparently many games' saves are too large to fit on a VMS without compression.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

Well, the thing is that I have an earlier save from Sam & Max which is "only" 56 blocks, so if >200 blocks (impossible to satisfy) is needed now that's a regression.

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Dec 3, 2018

When I experienced this problem, it was because I had compiled ScummVM without zlib support. Scumm saves tend to be about 100K in size, so they don't fit on a VMS with compression. When ScummVM is compiled with zlib support, it compresses saves down to the 10-20K range which does fit on a VMS. I wonder if your 56-block save was created with a build that was compiled with zlib support, and you're currently testing with a build compiled without zlib support.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

Hm, yes, you are right, unlike all my previous builds this one has

# USE_ZLIB = 1

in config.mk. Now I just need to figure out how that can have happened all of a sudden... I most certainly do have zlib installed.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

Ah, it seems that 7557f17 broke zlib (and MAD) on Dreamcast. It removed the hardcoded enablers, but the functions to check for zlib and mad use cc_check which fails to add $INCLUDES to the command line, so the headers (which are in $RONINDIR/incude) never get found...

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 3, 2018

@zeldin : You could just restore the hardcoded enablers, but if we do, can you add a comment in the configure script as to why that is needed. I think the "correct" solution would be to patch cc_check to respect the $INCLUDES ... but that could have side effects for other ports, but could try it and commit to see if the buildbot works fine at least... though not sure if that needs to be a PR.

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 3, 2018

@tsowell : This PR now has some conflicts due to commit 79a4e3f which touched the gui/options.cpp defines to remove the GP32 backend. I could probably resolve correctly, but if you could take a look and resolve that would probably be better :)

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

@digitall: Yeah, my local fix was to add $INCLUDES (and $DEFINES) to cc_check_no_clean, because that's how Makefile.common is going to compile stuff later so it is the environment it should test in. 😄
This fix makes detection work correctly for me and I was planning on pushing it, but if you want to test it on some other ports first you are very welcome. 🍰

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 3, 2018

@zeldin : That commit looks good to me. To be honest, I tend toward "publish and be damned" so I would just push it as probably the only way any minor port issues will get thrown up... and if they do they probably should be fixed anyway.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 3, 2018

Whelp. After re-enabling zlib I can save again, and the file is 47 blocks, so that's fine. However, now it seems like it's trying to create filenames for the savegames which are more than 12 characters long:
samnmax-1.s01, samnmax-1.s02 and so on. The VMU filesystem can't handle filenames longer than 12, so this gets truncated to samnmax-1.s0, all the saves go to the same file, and the load dialog can't find it... 😞

I suspect this is a regression caused by the new detector code - "samnmax-1" sounds like a gamename created by the detector to distinguish Sam & Max CD (which I swapped in) from Sam & Max Demo (on the boot disc, which I swapped out). All gamenames must be kept at 8 characters or shorter to prevent this issue.

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Dec 4, 2018

@zeldin, you're right - numbers are added to the end of domain names to ensure their uniqueness, and if a base gameid is already longer than 6 characters, it will exceed the 8-character limit (https://bugs.scummvm.org/ticket/10410). I have some ideas for how that could be fixed, but it's not going to be easy and is outside the scope of this PR.

For now I will try to duplicate the existing behavior to keep things consistent.

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Dec 4, 2018

@digitall, sounds good. I'll rebase and retest the PR once I've addressed @zeldin's feedback.

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 4, 2018

@tsowell : Sounds like a plan. Thanks.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 4, 2018

@tsowell : Yeah, even if some trick would be employed to handle >12 character filenames in general, we still need the savegame filenames to be consistent and not depend on which discs were swapped in/out before launch. Thanks.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 4, 2018

@digitall : Looks like I broke the mingw-w32 build on buildbot. It doesn't detect any libs anymore, but AFAICT config.log is not available in the web ui, so I don't see why exactly...
mingw does not use $INCLUDES, but it does use $DEFINES.

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 4, 2018

@zeldin : Will take a look ...

Loading

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Dec 4, 2018

@zeldin @digitall:

Latest config.log from the mingw-w32 build slave: https://gist.github.com/lotharsm/62da0c5995c0034c0481cd5c21efd2cd

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 4, 2018

Odd the breakage appears to be in building the test runner, hence why it only affects the native ports (since tests can only run on native... since only native binaries are runnable):
https://buildbot.scummvm.org/builders/master-mingw-w32/builds/460/steps/test/logs/stdio

This might clear after a full clean / ccache flush when the nightly runs... so will see if this resolves.

Loading

@digitall
Copy link
Member

@digitall digitall commented Dec 5, 2018

Even if it resolves, that doesn't mean it is detecting all libraries correctly, just that the build is not broken due to incremental build inconsistency.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 5, 2018

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 5, 2018

Hm, actually, on closer inspection, I think the problem is that -Dmain=SDL_main enters $INCLUDES from sdl-config --cflags, while the -lSDL2main needed for that to work goes into $LIBS, which is not added by cc_check_no_clean. Then the fix should be pretty straightforward (add $LIBS)...

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Dec 14, 2018

@tsowell: Thanks! I do agree with digitall that it would be nicer if you did not have to touch engine code.

I don't know if this is really important, but did you consider the case of a disc authored with a custom scummvm.ini on it? I think this used to work, provided you used the gameid as target, so it would be nice if it continued to do so. Not sure if anyone is actually using it...

Loading


static bool isIcon(const Common::FSNode &entry)
{
int l = entry.getDisplayName().size();
Copy link
Member

@ccawley2011 ccawley2011 Jan 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, commit b224b59 changed this to use hasSuffixIgnoreCase(). Is there any reason why this was changed back?

Loading

Copy link
Contributor

@zeldin zeldin Jan 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if there was a particular reason, but it doesn't really matter if the comparison is done ignoring case or not; filenames on ISO-9660 are always uppercase...

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Feb 6, 2019

Hi all, sorry to leave this hanging. While it should be easy enough to reproduce the existing save game naming behavior, it would be pretty messy. I think this should be shelved until the existing behavior is corrected.

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Sep 10, 2019

The best way to solve the file naming issues here would be to use a separate SaveFileManager for the DC backend, and override the openForSaving() method so that it creates the save file names as expected by the file system

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Sep 10, 2019

@bluegr The DC backend already has its own SaveFileManager. The issue here is that the filename sent to openForLoading et al is different after this change (and also dependent on which order you insert discs) than before.

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Sep 10, 2019

@zeldin: Right. The reason I missed it is that the DC's savefile manager does not inherit from the default one (inside backends/saves/default), and it resides in the dc backend directory (i.e. inside backends/platform/dc). Also, it is named differently (methods inside vmsave.cpp), and the methods are different as well (e.g. trySave(), tryLoad() etc). This should be adapted in the future.

It's not really efficient to adapt all the engines that are calling runModalWithPluginAndTarget() for DC. The savegame filename limitation for DC should be either in runModalWithPluginAndTarget(), or inside the vmsave methods, and not in engine code

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Sep 10, 2019

@bluegr Again, the real issue here isn't the savefile filename limitation. I know how to deal with that. The issue is that if we accept this PR then people will not be able to load their savegames because the filenames will change, and keep changing, in a manner that violates the principle of least surprise.

For example, if they have previously played a game with gameid 'foo', then they will have savegames with namesd like 'foo.s01'. But after the change ScummVM might instead look for 'foo-1.s01' or 'foo-2.s01' (and switch inconsistently between them on different runs, depending on how you start the game). These filenames can be handled just fine, but are not the filenames used previously so the user won't find their saves.

That is what the changes to the engines are trying to fix, and while I agree that it is not the correct way to fix it, it don't quite follow what the vmsave methods could do about it.

Loading

@bluegr
Copy link
Member

@bluegr bluegr commented Sep 11, 2019

For example, if they have previously played a game with gameid 'foo', then they will have savegames with namesd like 'foo.s01'. But after the change ScummVM might instead look for 'foo-1.s01' or 'foo-2.s01' (and switch inconsistently between them on different runs, depending on how you start the game). These filenames can be handled just fine, but are not the filenames used previously so the user won't find their saves.

This naming scheme is used in other ports, too. Since this port has such limitations for saved game names, these should be handled in the code which performs saving/loading.

That is what the changes to the engines are trying to fix, and while I agree that it is not the correct way to fix it, it don't quite follow what the vmsave methods could do about it.

You could change trySave() so that it ignores the filename parameter, and replaces it with ConfMan.get("gameid"). Same with tryLoad(), and associated methods. I agree that this isn't optimal, but at least it handles the DC filename limitations in a central place

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Sep 11, 2019

@bluegr You are consistently missing the point. The main issue is not with filename limitations, but with the fact that a suffix is added to differentiate between different "installations" of the game. This works fine if the mapping between suffix and "installation" is kept constant in nonvolatile memory (which is what happens on other ports), but on the Dreamcast there is no location where such a mapping can be reliably stored. Before this PR only one installation was added to the (volatile) config database (after the custom selector had done its thing), so a suffix was never added. Thus the same installation would always get the same suffix (i.e. none). But by using the regular selector, installations are added to the (volatile) config database as detection happens, which means that not only can the games get suffixes, they can get inconsistent suffixes depending on the order in which installations were added to the config database.

I think the issue would be mostly resolved if the database was cleared (and reloaded from scummvm.ini on the CD if one exists) on disc swap. Then the only change would be that if you have two installations with the same gameid on the same CD, they would no longer share a savegame pool. It could still make old savegames unusable with things like the dual language Loom CD though, in case the "wrong" installation gets the suffix.

Just replacing the filename with ConfMan.get("gameid") is obviously not going to work because a game can have more than one savegame so the gameid/targetid is just one part of the filename. Trying to parse the given filename to isolate the different parts and replace them selectively could ostensibly work, but seems somewhat fragile...

Loading

@sev-
Copy link
Member

@sev- sev- commented May 3, 2020

It seems that this nice PR got kind of stuck.

One of the ways of addressing saves would be to transparently truncate it, or even use first 2 characters, followed by CRC32 of the filename (without extension), thus, samnmax-1.s01 becomes sa22D74833.s01 and samnmax.s01 stays as sa5B280160.s01. Then, when you need to scan a directory for saves when querying them for enlisting in GUI, you will have a consistent way of replacing filename with proper CRC.

The only obvious disadvantage is that you losing the possibility to take the save from DC and move it to your desktop as is, but something tells me that this process is so special and requires additional hardware, so it is not practical.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented May 3, 2020

@sev- A much more severe disadvantage of your proposal is that you lose the possibility of continuing a game that you saved on the DC with an earlier version of ScummVM... It is this possibility I've been saying all along that I want to keep...

Loading

@sev-
Copy link
Member

@sev- sev- commented May 4, 2020

@zeldin, I do not understand what you're talking about:

 openSaveFile(char *s) {
    Common::String newname = genFileName(s);
    if (file.open(s)) {
        renameSavefile(s, newname);
        file.close();
    }
    if (!file.open(newname))
        return NotFound;
    ...
}

And saving is always with generated file names.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented May 4, 2020

Ok, sure, you can rename old saves to the new scheme, but then you instead lose the possibility of going back to the old scummvm version. Not to mention that the "rename" function would have to be implemented in the first place... 😉

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented May 4, 2020

And anyway it's not solving the actual problem, which is that s will have the wrong contents ("samnmax-1.s01" instead of "samnmax.s01"), so file.open(s) would fail.

Loading

@sev-
Copy link
Member

@sev- sev- commented May 4, 2020

@zeldin, I do not understand this last comment. I mean, no previous version could ever create samnmax-1.s01 file, right? So, we will not have older saves, only newer.

Well, you could also use this scheme only in case your base filename exceeds 8. Thus, you will be able to have the previous saves untouched, and new ones, with longer names, created in a compatible way.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented May 5, 2020

@sev- Since you seem to have only skimmed the previous discussion, I'll summarize it for you. There are two related but different issues at play here.

#1: This PR changes the way the GameDomain is set for games. With the old selector, the GameDomain would always be the gameId. So even if you had three variants of Sam&Max, two on one CD and on on another CD, the GameDomain would always be samnmax regardless of which one of them you launched. So savegames would be called samnmax.s01 and so on. While having overlapping savegame names for three instances of the same game might not be ideal, you can work around it by using different memory cards for the three.

With this PR, the GameDomain for these three instances would be samnmax, samnmax-1, and samnmax-2, and the savegames for two of them would suddenly change to samnmax-1.s01 and samnmax-2.s02, making the old saves inaccessible. This is a regression, and applying some kind of hash to reduce the length of samnmax-01.s01 will not fix that, you need to look for samnmax.s01 because that is the filename the old save will have.

Also note that the assigment of which instances get the -1 and -2 suffixes is volatile; if you insert the CD with two instances first and then the CD with one instance, then the latter will be called samnmax-2. But if you power cycle the system and then insert the second CD first, the instance on that one will be called samnmax. So newly created savefiles are also hit by this issue.

#2: If a game with a GameDomain longer than 8 characters is launched, then the savegame filenames will exceed the limit of 12 characters, and saving will not work. This is not a regression; the old launcher did not perform a length on the gameId, so if it was something like discworld, that would break. This would be nice to fix, but is not directly related to this PR. Issue #1 merely exposed this issue by creating a longer GameDomain than 8 characters for a game where this used to not be the case.

After some thinking I'm beginning to lean towards a KISS solution here - simply remove the regexp -[0-9]+ from the filenames in the SaveFileManager. It's a bit hackish, but as long as no gameId contains a dash followed by a number (such as monkey-2), it should work. If the filename is still to long after this stripping, then some hashing transformation could optionally be applied to address the second issue.

This assumes that engines will be ok with finding filenames such as samnmax.s01 and discw~a7.s01 in the result when they call listSavefiles("samnmax-01.s*") and listSavefiles("discworld.s*").

Loading

@sev-
Copy link
Member

@sev- sev- commented May 5, 2020

I see. Although I read the whole thread, somehow I missed it. Thanks for the sum up.

In this case I would add a platform-dependent code to EngineManager::createTargetForGame() in plugins.cpp:577, something like If it is DS, and a target exists, then remove the previous one.

As a result, if two variants of a game will sit on same CD, only the last one will be accessible, but I guess it is how it was working before.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented May 5, 2020

Actually, having two variants on the same CD did work before. The trick is that no Target/GameDomain is created until after the user picks the game to launch (and then only for that specific instance). Before that only EngineMan.detectGames() is used, so there is no contention between multiple instances.

Loading

@henke37
Copy link
Contributor

@henke37 henke37 commented Jun 29, 2020

Would there be a point in waiting with this PR until after the GSOC project to redo the metaengine parts of the plugin system?

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Jun 30, 2020

@henke37 Possibly, if the plan is also to refactor the savegame handling. Right now there is two problems with solving this in the MetaEngine:

  1. Not all engines use MetaEngine (by way of GUI::SaveLoadChooser) for savegame handling
  2. The engine can pass a non-NULL string pointer to override the basename chosen by MetaEngine

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Jul 6, 2020

@tsowell I have some time to work on this now, but it looks like the changes do not apply cleanly to master anymore. Do you think you could take a look at fixing the conflicts, and I'll try to do something about the savefile filenames in the backend.

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Jul 6, 2020

I now have a workaround for backends/platform/dc/vmsave.cpp which makes old saves work without commit 1d159cd. It's a little bit hackish, but it works (although it will break if you have more than 99 savegames in myst, because there is no way for it to know that myst-100.mys is not a save for the 100:th installation of myst, which should have been myst.mys by the old rules)...

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Jul 7, 2020

@tsowell I have some time to work on this now, but it looks like the changes do not apply cleanly to master anymore. Do you think you could take a look at fixing the conflicts, and I'll try to do something about the savefile filenames in the backend.

Hi @zeldin, sorry, but I don't know when or if I'll have time to spend on this. I don't even have my notes for creating a build environment for this backend anymore, so I'd be starting from the very beginning.

Loading

@sev-
Copy link
Member

@sev- sev- commented Nov 1, 2021

Do we have any update for this PR?

Loading

@zeldin
Copy link
Contributor

@zeldin zeldin commented Nov 3, 2021

@sev- I have not heard anything more from tsowell.

Loading

@tsowell
Copy link
Contributor Author

@tsowell tsowell commented Nov 15, 2021

Sorry if you've been waiting on me! I'm afraid I don't have time to work on this. Feel free to close it if no one else can pick it up.

Loading

@sev-
Copy link
Member

@sev- sev- commented Nov 23, 2021

It is a pity. I'm closing this, but please do not purge your branch. Perhaps someone will pick it up in the future.

Loading

@sev- sev- closed this Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants