Use “Enable game-specific enhancements” for Sierra bug fixes

[obskyr]

As of some recent version(s), bugs present in the original version of a variety of Sierra games were fixed in ScummVM, making their behavior different from the originals. Following the precedent set by ​these LucasArts games, these bug fixes should use the “Enable game-specific enhancements” setting.

I don't know exactly which games are affected, but ​this blog post mentions “over one hundred original Sierra bugfixes” being added in version 2.1.0. It's probably possible to find the relevant games and changes by looking at the commits! It seems that most (if not all?) of the changes are in ​/engines/sci/engine/script_patches.cpp – perhaps this whole file can be disabled, rather than having to do it per enhancement.

I've filed this ticket under “Engine: SCI”, but it might affect some AGI games as well.

---

I'm not sure what the stated purpose of ScummVM is – I believe it's primarily to recreate these games so that they're accessible on modern platforms. Going back and changing their behavior, though, suggests that it might be something more akin to “making a better version of these games”. On the other hand, there's the aforementioned “Enable game-specific enhancements” checkbox, so it might be both!

On a personal note, I use ScummVM for convenience, and bar concessions made for that convenience, want to experience these video games as they were to the extent that that's possible. For example, I recently played the MS-DOS version of The Colonel's Bequest, and was disappointed to see ​the plane flying by, as opposed to the hilarious and charming bug that that makes a statue fly by in that version of the game.

[m-kiewitz]

We normally really only fix bugs, that are obviously bugs, not change game behavior.
Changing games in that aspect is not the intention of ScummVM, and should be done using (external) script patches, which we support and everyone can use. If we would include such a game changing patch, it should be optional of course and use such a game option.

"perhaps this whole file can be disabled, rather than having to do it per enhancement."

This wouldn't really make sense, almost all of them if not all of them are actual bug fixes.
Even as a non-developer you can easily read the comments about every single script patch and it's detailed. Please do that.

For example the French version of King's Quest 5 crashes (hangs) almost at the end. Without the script patch, you would get the "true experience" of the game basically freezing, with no way to recover. Maybe you want that, I don't know, but most players do not want that, because it's simply broken. I think when I fixed that one, a speed runner was even happy that it was fixed, because he was finally able to use that version to speedrun the game, which was impossible to use before.

Or take Gabriel Knight 1, which has a few game speed bugs, that happen when the computer running the game is too fast. It causes certain sequences to become unbeatable/unplayable. Disabling that once again wouldn't even give you the "true experience", unless you played the game late and it was unplayable and you want to re-experience that.

There is also a major script bug in Quest for Glory 1 VGA at the end, in the crazy room with Yorick. When you walk to certain spots, the game will go haywire, the graphics will fully glitch and the game will basically freeze. Granted, the graphic glitch has to be seen and it looks funky, but for a player the game is simply broken.

Please also understand that SCI within ScummVM can not even really give you the "true" SCI experience, because SCI - the original interpreter - written by Sierra, did almost no error checking. Scripts were also not checked for certain errors. So when something went wrong, or when something was programmed wrong or badly, the interpreter just went on and on and then maybe fully crashed 1 hour later. Or broke memory that ruined another part of the game. Or it simply worked by accident. The SCI code within ScummVM does not do this, but instead checks for all sorts of error conditions. It also does proper memory handling. We see that as a feature, as something good, because no one probably wants their game to crash on them, to hard lock or to break or have memory corruption. Quest for Glory 3 for example will crash after a while simply because it runs out of memory, when using the original interpreter (depends on how much you do in the game).

IF we disabled all these error checks and verifications, you would in fact even get more game bugs, because some bugs simply did not trigger any problems in the original interpreter by pure chance, while in our interpreter it does because we don't re-use memory and clear it before using.

And we also patch in optional features, like audio + subtitles at the same time. This is optional, you don't need to use it. Maybe you don't even want a new selection for that (King's Quest 6 didn't even have a selection for that variant, because it was not supported, and I personally created a graphic and inserted it into the game for its options menu. Originally you could only select audio or text, now you can also select "both". You have to click on that option to even see that new graphic.

AGI engine btw. does not have such a patch engine (yet).
AGI engine offers a few features that the original interpreter did not offer, like for example Hercules-style pausing when you enter a command outside of Hercules graphics mode. These are optional and you have to enable them yourself.

There are also workarounds, see the file workarounds.cpp.
Same deal there, if you disabled these, you would have all sorts of game breaking glitches.
Maybe you want these, I don't know. But players typically don't want their games to have game breaking glitches/crashes, which is why we try our best to fix these issues.

[obskyr]

Alright – script_patches.cpp can't simply be disabled. That's good to know! It's why I qualified that suggestion with a “perhaps”. I'm just a lowly user: unlike you, I don't know the codebase.

But that's only to speak of game-breaking bugs and crashes. That's apparently not the extent of the changes – see, for example, the aforementioned Laura Bow bug. The fact remains that there's an “Enable game-specific enhancements” checkbox because people wanted behavior more like the originals. Sierra games should have that checkbox in the same way LucasArts games already do. Whatever line you draw there between “game-breaking bug fix” and “enhancement” isn't my business – make that judgment as you wish!

Since it's all in this one file and limited to the SCI engine, it might not be as difficult to find the changes as I thought! Perhaps a pull request would be welcome? I'm envisioning adding a boolean “essential” flag (which signifies whether something is game-breaking or not) to each patch, and unchecking the “Enable game-specific enhancements” checkbox would then only disable patches where it's set to false. I'd also update ​the wiki article to reflect which patches are included.

[m-kiewitz]

I'm sorry, but I don't know which Laura Bow bug you are talking about.
I don't see a script patch for the Sleuth-O-Meter. Several game bugs are mentioned in that blog though.
The blog simply states all the things that are going wrong and how to get the top rating in the game. That is all, as far as I can see. It states several things that were not well programmed, but it doesn't seem that we even try to fix that and it would be a major undertaking to do things like these anyway.

In Laura Bow an Easter egg also gets fixed that was using the wrong graphic, but was fixed in other versions by Sierra. We consider that a bug too. It gets difficult for these kind of bugs, but at least in that case it was fixed by Sierra, so it makes sense for us to fix earlier versions.

Most script patches for Colonel's Bequest fix lockups/hangs and also a bug that rejects a correct copy protection answer.

There is one bug fix, which fixes an issue, where you can't interact with Lilly in act 4 in the DOS version, which was once again fixed in other versions by Sierra, so we take that as proof that it is a bug that should get fixed.

[obskyr]

I'm talking about the bug I linked to ​a video of in the original ticket – the easter egg you mentioned. It's currently on line 8991 in script_patches.cpp. That'd fall under “Content restoration”, like ​the enhancements inspired by earlier/later versions in Monkey Island 1 and 2 (Ctrl-F “content restoration”).

But alright, I'll consider putting together a pull request for this, then, if I have time. I'll use “is this game-breaking / a crash or not?” as a yardstick.

[m-kiewitz]

I disagree.
It's not content restoration, it's a bug.
The easter egg is there, it simply uses the wrong graphic.

We don't add the easter egg in there. We also don't add the graphic in there. That was done for these content restorations in SCUMM.
The easter egg script is simply buggy and what it does in the DOS version makes no sense.

Sierra fixed it after releasing the DOS version and we simply do the same for the DOS version.
If we added that easter egg to DOS, or enabled a disabled easter egg, it would be a different story. We don't do either of these things and I personally would even refuse to do so.

It's a typo for the graphic that is used. That is all.
And someone even seeing the broken easter egg in DOS may assume that it is a game bug that is doing that and not an easter egg, because it looks broken when using the wrong graphic. Like a script glitch. I think I even saw a video where this was mentioned for the DOS version, that it looks like a game bug.

[obskyr]

Well, whatever you want to call it, it's a change in behavior from the original that isn't an essential fix. You can call it a “bugfix” – some of those are included in that wiki article as well, as you'll notice.

Currently, the ScummVM version of the MS-DOS version of The Colonel's Bequest doesn't match any version of the game that was actually released, and in fact, it's impossible to view this funny, inconsequential bug in a current version of ScummVM at all. A person who unchecked the “Enable game-specific enhancements” box would expect that behavior to be restored, so whatever you may call it, that's the sort of thing the checkbox is for.

[m-kiewitz]

The graphical glitch in Quest for Glory 1 VGA that I already mentioned is also funny, it's still a glitch and we patch it out and thus you can not see it in ScummVM either. Maybe I could even change the script patch, so that the game doesn't lock up afterwards, but the graphical glitch still happens. Still I wouldn't see the point of it.

Sierra fixed the glitch in every other version of Colonel's Bequest and it looks like a script bug and I think it's wrong to hide such a bug fix under "game enhancements", because the average user will not enable that, may see this glitch (it happens on random chance, it's not like you have to do some secret combination of actions to get it) and report it as a game bug, which it is. So in the end we will have a bug report about this, which will say "please enable game enhancements to see that it's actually an easter egg", which is in my opinion ridiculous.

I would rather implement a Colonel's Bequest specific game option to get this glitch instead.
In fact we could even do a script patch that breaks it in all versions, so that someone who wants to experience it doesn't have to own the DOS version.

Again: if it was disabled in DOS, and we would enable it, that would be a different story. Or if we added it in DOS. But we don't. That's what was done for these content restorations.

Sierra cared enough about it that they fixed it themselves in every other version and I think one should acknowledge that.

[obskyr]

From what I can tell by the wording of the wiki article, the “Enable game-specific enhancements” box is checked by default, no? So you won't get any bug reports like that from the average user. No more than you would get bug reports for the fixes of weird behavior that are already behind that checkbox for SCUMM games.

I filed this ticket not because I want this one specific bug to be restored, but because I want to make sure that any Sierra games I play via ScummVM in the future have original bugs (at least non-breaking ones) intact, so that I get to experience them. I'm not interested in putting a new bug into versions that didn't have it, and can't imagine anyone would.

[m-kiewitz]

"I don't know exactly which games are affected, but ​this blog post mentions “over one hundred original Sierra bugfixes” being added in version 2.1.0"

This is a factually incorrect claim.
I created the SCI script patcher used in ScummVM ages ago, in 2010. More than 10 years ago.

What the blog says is that there is a new ScummVM version from October, and that it contains over hundred Sierra bugfixes written by sluicebox, not that hundreds of Sierra bugfixes would have been added in that specific version. Sluicebox has been working for this project for years now.

And the Laura Bow easter egg script patch was added in 2016 and back then someone even first figured out that it's not a script bug, but instead an (unknown) easter egg using the wrong graphic. I can remember it on IRC.

[obskyr]

…Why are you going back and picking out random incidental things to argue against?

[m-kiewitz]

Replying to obskyr:

From what I can tell by the wording of the wiki article, the “Enable game-specific enhancements” box is checked by default, no?

If it is enabled per default, then that's another matter.
I would have to look into that.

I filed this ticket not because I want this one specific bug to be restored, but because I want to make sure that any Sierra games I play via ScummVM in the future have original bugs (at least non-breaking ones) intact, so that I get to experience them. I'm not interested in putting a new bug into versions that didn't have it, and can't imagine anyone would.

Well, another question is what is non-breaking bugs?

Missing points should be kept?
What about music volume issues?
Unresponsive controls (but still working in general, see QfG1 VGA combat)?
There is also special script code that Sierra did for the intro of Freddy Pharkas, where in ScummVM you don't see it scaling in, simply because ScummVM doesn't display new frames all the time, and because of that inner loop you can't see it scaling up anymore. That doesn't break the game.
What about audio + text at the same time, when it wasn't possible originally? Disable that too, despite there being an option to disable that/not enable it?
What about new graphical option for both in KQ6? That's non-breaking, but removing it would mean you can't play the games "without enhancements" but with audio + subtitles anymore.
Dialog getting cut off in Gabriel Knight 1 (one would have to test how that worked on slow hardware, did it play fully on slow hardware? If so, then what's the reference? What a slow machine did, or what a fast machine did?)

Some are simple, but for some it gets really complicated.

[m-kiewitz]

Replying to obskyr:

…Why are you going back and picking out random incidental things to argue against?

That's a major part of ticket description, and I'm just saying that this part is factually wrong and a misunderstanding. That's all. Should probably get removed, it's not even really what this ticket is really about.

[obskyr]

Replying to m-kiewitz:

Replying to obskyr:

…Why are you going back and picking out random incidental things to argue against?

That's a major part of ticket description, and I'm just saying that this part is factually wrong and a misunderstanding. That's all. Should probably get removed, it's not even really what this ticket is really about.

Sure. I've changed it to “having been added as of” instead of “added in”.

[obskyr]

Seems pretty clear-cut to me. With enhancements disabled, all of those should be accurate to the original, except the things that are already options (like audio + text and the KQ6 graphical option), since you can toggle those separately. As for the Gabriel Knight one, ScummVM sets out to fix processor speed issues, so dialogue might as well play out fully.

So what's the word? Provided that:

• Enhancements are enabled by default,
• Patches are excluded from the option if they fix crashes or game-breaking bugs, make the ScummVM version more accurate to the original (like Freddy Pharkas), or have a separate option (like the KQ6 option), and
• Which patches are included is documented on the wiki

Would a pull request to add this option be welcome? I don't want to put in the work if it'd only be declined.

[m-kiewitz]

I have to talk with sluicebox about it, and also go through all the patches first.

And I would say most patches do fix game breaking things, so adding some flag about patches being needed doesn't really make sense, instead it should be the opposite. "Enhancement"-flag, or idk.

What about Space Quest 4 Skate-O-Rama fixes? You can call these game-breaking, but even in Original SCI there were ways to get through, it was simply a nightmare.

One really has to go through everything one-by-one.
And what would make sense is also to create a wiki about all these fixes.
I also just see that sluicebox added tons of QfG4 script patches, he knows these best.
When we are already going through them all anyway...

[dwatteau]

Hi,

Speaking as a contributor to the SCUMM enhancements which were mentioned several times here:

• The various game engines supported by ScummVM are often maintained by different teams, and each engine has a different technical approach/background and has to deal with its own realities (different limitations, philosophy, player expectations, resources...).
• ScummVM doesn't make a general promise on total accuracy. It's often a best-effort, with a lot of work being done, that's for sure. But if an engine concentrates on accuracy, or improvements, or tries to provide both, that doesn't mean that a different engine must do the same. For users, I agree that consistent behavior between engines is nice -- but again, no promise.
• (What the SCUMM engine does with this "Enable game-specific enhancements" setting is still a bit of a work-in-progress playground, at the moment. Even in the SCUMM engine itself, we don't know if that's the best approach yet: some users may want a finer granularity over the different types of enhancements/bugfixes that we provide, but we don't if/how this is going to happen yet. Other engines handle this differently, as for Blade Runner.)
• As far I know about Sierra games (I know very little about them, so I may be wrong), they require an order of magnitude more bugfixes than the LucasArts titles, and they're often quite important (for the reasons given by m-kiewitz above). In comparison, most of the original bugs/oversights that we fix in SCUMM are kinda minor (i.e. we find some very small script errors, and we fix them when we're almost sure that the authors intended a different behavior). We do fix some game-breaking SCUMM bugs, and those are hardcoded, but I think we only have a handful of them.
• That metric is important; dozens of relatively minor enhancements (SCUMM) require less maintenance than dozens (hundreds?) of important bugfixes (SCI). Every time you add an ON/OFF button to a bugfix, you need to test both paths. And if you add a new bugfix, or fix a different part of the engine, you may need to test all your bugfixes/enhancements again, especially if they have side effects... So if's up to the engine maintainers to decide if it's worth it -- they know their context, and they're the ones who do the (free and volunteer!) work. It's not just about adding a flag, but it's also about maintaining it.

[bluegr]

The script patches we have created for the SCI engine are vastly different from the ones in the SCUMM engine, as almost all of them lead to game crashes, and are not cosmetic bugs.

These patches have been created for the following reasons, among others:

• To fix bugs that worked in Sierra SCI purely by accident
• To fix script bugs that result in lockups, crashes or invalid game state
• To fix timing bugs in game scripts that worked only in old PCs. Modern PCs that are much faster trigger all sorts of timing bugs
• To fix serious bugs in unpatched game versions that were later fixed by Sierra, and result in crashing, lockups or unintended dead ends
• To fix badly written scripts in fan games
• To increase the number of allowed saved games
• To fix compatibility with some fan patches (e.g. the fan made subtitle patch in GK2)
• To fix missing points in some games, that were explicitly mentioned in the official Sierra walkthroughs (e.g. in LSL2)
• Add functionality for simultaneous speech + subtitles
• Disable the change directory button in original save/load dialogs (we place all saved games in the same folder)

Of course, there are tons of script patches, and some which belong to other categories that I may have missed.

NONE of the above fixes change the way the actual games work. None can be toggled by the user, as disabling any of these patches would crash the game. It doesn't make sense to disable any of the patches. In this bug, a single easter egg is referenced, which was initially implemented incorrectly, and later fixed. There is no point in allowing the user to toggle the bad implementation, just to see a flying statue instead of an airplane.

So, to sum up: we can't, and won't allow toggling of the patches that are implemented in the SCI engine, as almost all of them lead to crashes, lockups and unintended dead ends. The only ones which would be considered as toggleable in the future are the fixes which would change the way the games are played. To my knowledge, we have no such fixes up to now. In the future, if we have such patches, we will consider to allow users to toggle them.

Thus, for now, this discussion doesn't lead anywhere: we have closed multiple game bugs, and we don't want to allow users to re-enable them, thus I'm closing this bug as invalid. If there is a compelling reason to discuss or toggle specific script patches that we do in the SCI engine, feel free to open a feature request.

Closing as invalid