1
SCI Development Tools / Re: SCI Companion V3 - alpha build notes/bugs/feature requests
« Last post by gumby on Yesterday at 10:38:34 AM »Thanks Kawa.
Recent Posts
2
SCI Development Tools / Re: "Sounder" tool - alpha version is ready
« Last post by Collector on August 12, 2022, 07:10:39 AM »404 from your link, but I found it on your repo.
3
SCI Development Tools / Re: SCI Companion V3 - alpha build notes/bugs/feature requests
« Last post by Kawa on August 11, 2022, 05:26:40 PM »Just a heads-up, I just fixed some crap involving file dialog filter lists having random garbage, and an uncaught exception causing a CTD involving the VGA Picture palette editor and not having any image commands. Well, it's not much of a fix, but at least you'll not lose any work to a CTD and you'll know why. Also includes a tiny fix in the pattern pen bitmaps from a few months back.
Latest build is in the stash in my signature as always.
Latest build is in the stash in my signature as always.
4
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by 8bitKittyKat on August 11, 2022, 10:17:08 AM »Of course, but ScummVM also adds their own internal patches to SCI script bugs so you won't have to manually patch your game.Ah, I didn't know they had their own patches in the engine. Personally I use DOSbox 99% of the time. I'm not sure I want to add additional work by writing a patch for ScummVM too, especially since I'm not done yet and this bug is proving to be stranger than I first thought, but my github repo is public and if when I'm finished someone wants to port my work, that's fine by me.
You can take a look at one such fix: https://github.com/scummvm/scummvm/commit/87afecfe1394398c18324ec06dc628eb2e923e9d
It's a bit involved to fix it in ScummVM but you can try seek the devs (especially SluiceBox) on discord for help.
5
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by OmerMor on August 11, 2022, 09:51:21 AM »ScummVM works with SCI patch files, which is what's I've included in the zip, so it should just work if you put the files in the game folder.
Of course, but ScummVM also adds their own internal patches to SCI script bugs so you won't have to manually patch your game.
You can take a look at one such fix: https://github.com/scummvm/scummvm/commit/87afecfe1394398c18324ec06dc628eb2e923e9d
It's a bit involved to fix it in ScummVM but you can try seek the devs (especially SluiceBox) on discord for help.
6
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by 8bitKittyKat on August 11, 2022, 07:00:58 AM »Just did some quick testing and it's looking like the bug isn't in v1.000 of Hero's Quest. I don't have a decompilation of that version aside from what I can get through SCI Companion's tools unfortunately.
That's a very interesting bug!ScummVM works with SCI patch files, which is what's I've included in the zip, so it should just work if you put the files in the game folder.
Have you considered writing a patch for ScummVM as well?
7
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by OmerMor on August 10, 2022, 03:24:25 PM »That's a very interesting bug!
Have you considered writing a patch for ScummVM as well?
Have you considered writing a patch for ScummVM as well?
8
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by 8bitKittyKat on August 10, 2022, 02:55:45 PM »When I started working on my bug fix I did a search for any scripts with "if (CastSpell spell)" outside of a switch and restructured those bits of code, so yeah, I probably didn't need to every instance. I'll go through and see what rooms were unnecessary. Also I did see the redundant spells in some rooms.. I had a bit of an itch to tidy it up when I saw it, but I had to remember I'm trying to patch a bug and not needlessly rewrite a whole bunch of code! That would be.. a whole other beast.
Thanks for the thorough explanation. I'm very new to SCI and still a novice programmer so it's appreciated. I'll be fixing up the EGA patch soon and doing more testing on it. Then on to the VGA patch.
Thanks for the thorough explanation. I'm very new to SCI and still a novice programmer so it's appreciated. I'll be fixing up the EGA patch soon and doing more testing on it. Then on to the VGA patch.
9
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by Charles on August 10, 2022, 11:44:10 AM »I've looked pretty thoroughly through the code now, and I think I know exactly what's going on. Some of this might help you fix the VGA bug, because it's probably the exact same thing going on there.
It's not effecting all spells in the rooms you've identified. It's not even actually causing problems in all the rooms you fixed, so you can scale back your fixes a bit. That said, I think your fix is the best way to do so. The basic problem is exactly as you found -- the (switch spell) is happening after (CastSpell spell).
The basic order of things is that in Main, the glory::handleEvent procedure runs constantly, and for speechEvents (of which typing "Cast anything" is one) it tells the specific room to do what it needs to do first:
So, what should be happening is that the room specific stuff should check only for spells that do something unique in that room, and if so cast the spell (via (CastSpell), do the animations, whatever, and claim the event.
Instead, what's happening is that those buggy rooms are casting the spells first thing with a (CastSpell) call -- that improves your skill with the spell, drains your mana, and flags the event as claimed, so nothing else will try to claim the spell -- then it's checking if it should actually do anything specific with the spell. If so, do that animation, show that text, otherwise if it's a spell we're not doing anything special with reset the event claimed flag back to FALSE, so the main script can deal with it.
But we're already cast the spell... so we're casting it twice. Once in secret, then finally with the animation and text.
There's two ways to fix it... one, as you've already done is to swap around the switch statement and the CastSpell statement. So you check to see which spell the player typed in, and only CastSpell if it's one this room recognizes... otherwise, leave the claimed flag as false and let the main script handle it. You can see the original programmers already did this in some rooms, like the Stags in Rooms 77 and 78.
The other way is to add cases for all 7 spells, and don't let the main script handle any of that. That's actually the approach done in the RandomEncounter.sc script. So that room you didn't actually need to switch the order around, because it's not leaving the event unclaimed for the main script to handle. It's not my favorite approach though, because it means needlessly copying code into all the rooms. Something the original programmers did a fair bit... there's like 5 places where they handle the Zap spell, and it does the exact same thing in each... scratch that, almost the exact same thing. They slightly changed the text for when you're not holding a weapon in all but the Main script.
Best I can figure, there were 31 separate instances that check if the user tried to cast a spell (calls to the SaidCast function), not counting the one in Main. So to be sure, I think you'd need to check each of those rooms, and see if they're casting the spell first, then accounting for a fraction of the spells then flagging the event as not claimed. Those would be your problems rooms, and I suspect it'll be the exact same in the VGA version.
It's not effecting all spells in the rooms you've identified. It's not even actually causing problems in all the rooms you fixed, so you can scale back your fixes a bit. That said, I think your fix is the best way to do so. The basic problem is exactly as you found -- the (switch spell) is happening after (CastSpell spell).
The basic order of things is that in Main, the glory::handleEvent procedure runs constantly, and for speechEvents (of which typing "Cast anything" is one) it tells the specific room to do what it needs to do first:
Code: [Select]
(super handleEvent: event)
If the room code has done what it needs to do, it will set (event claimed: TRUE). I.e. it did the job, stop there.So, what should be happening is that the room specific stuff should check only for spells that do something unique in that room, and if so cast the spell (via (CastSpell), do the animations, whatever, and claim the event.
Instead, what's happening is that those buggy rooms are casting the spells first thing with a (CastSpell) call -- that improves your skill with the spell, drains your mana, and flags the event as claimed, so nothing else will try to claim the spell -- then it's checking if it should actually do anything specific with the spell. If so, do that animation, show that text, otherwise if it's a spell we're not doing anything special with reset the event claimed flag back to FALSE, so the main script can deal with it.
But we're already cast the spell... so we're casting it twice. Once in secret, then finally with the animation and text.
There's two ways to fix it... one, as you've already done is to swap around the switch statement and the CastSpell statement. So you check to see which spell the player typed in, and only CastSpell if it's one this room recognizes... otherwise, leave the claimed flag as false and let the main script handle it. You can see the original programmers already did this in some rooms, like the Stags in Rooms 77 and 78.
The other way is to add cases for all 7 spells, and don't let the main script handle any of that. That's actually the approach done in the RandomEncounter.sc script. So that room you didn't actually need to switch the order around, because it's not leaving the event unclaimed for the main script to handle. It's not my favorite approach though, because it means needlessly copying code into all the rooms. Something the original programmers did a fair bit... there's like 5 places where they handle the Zap spell, and it does the exact same thing in each... scratch that, almost the exact same thing. They slightly changed the text for when you're not holding a weapon in all but the Main script.
Best I can figure, there were 31 separate instances that check if the user tried to cast a spell (calls to the SaidCast function), not counting the one in Main. So to be sure, I think you'd need to check each of those rooms, and see if they're casting the spell first, then accounting for a fraction of the spells then flagging the event as not claimed. Those would be your problems rooms, and I suspect it'll be the exact same in the VGA version.
10
The Games and other Sierra Adventure stuff / Re: Found a bug in QFG1 EGA and VGA.. (and fixed it)
« Last post by 8bitKittyKat on August 09, 2022, 10:58:40 PM »That is really awesome!
I hope to be able to take the time and really see what’s going on. If I had to guess I’d say for the EGA one, the room-specific code isn’t claiming the Said event as finished, so it runs the room specific code first, then runs the generic code, thus double-casting the spell.
Amazing find, and fix! Great work.
Cool find. Here's what I see in the buggy QfG1EGA code.I was looking at the code again earlier because apparently I missed a couple rooms, and was beginning to think it might be something like that. Thanks for confirming it!
When "(if (CastSpell spell)" is used before "(switch spell", it's always casting the spell, even if there isn't a switch case for the spell the player typed. So it gets cast twice in these rooms, once by the (CastSpell spell) procedure and then again in Main's default spell handling. Any spell that does have a switch case in the buggy rooms, like (DETMAGIC in bearCave, *should* only consume mana once. This is because the event gets claimed, not sent back to Main for handling like spells that aren't matching a switch case do.
SMF 2.0.19 |
SMF © 2021, Simple Machines
Simple Audio Video Embedder
Page created in 0.108 seconds with 17 queries.