SCI32: Transitions (kSetShowStyle, kSetScroll) #791
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csnover
changed the title
| @@ -225,9 +225,11 @@ class SegManager : public Common::Serializable { | |||
| * @param[in] size Number of bytes to allocate for the hunk entry | |||
| * @param[in] hunk_type A descriptive string for the hunk entry, for | |||
| * debugging purposes | |||
| * @param[in] gc Whether to make the hunk eligible for garbage | |||
| * collection | |||
There was a problem hiding this comment.
IMHO, it would be more preferable to store persistent hunk entries in a separate segment type (e.g. PersistentHunk), instead of changing the behavior of the Garbage Collector. Such entries can then be persisted in saved games, as well (like dynamic text objects).
|
Great work overal, well donel! |
|
OK, added the bitmap segment. Any thing else? |
csnover
force-pushed
the
sci32-transitions
branch
from
3569e41
to
8851a2d
Compare
Closed
|
Looks good! |
csnover
force-pushed
the
sci32-transitions
branch
from
b690a62
to
a23ec56
Compare
Bitmaps in ScrollWindow and Robot code are managed by the kernel and not by game scripts, although they must be able to be referenced through a reg_t. To prevent incorrect GC of bitmaps that are in use but not referenced by any game script, explicit memory management of hunk entries can be enabled.
This commit implements all of the known plane transitions from SCI2 through SCI2.1mid games. Because kSetShowStyle is always called indirectly via the Styler game script, it is difficult to find all the places where transitions are used. As such, transitions that appeared to never be used have been added as stubs which will trigger a game crash with a message to report what was being done, so any missed transition types can be identified quickly and then implemented.
This is used by Torin in room 50900.
In the original engine this would have simply resulted in no draw, but ScummVM is more strict about it. It is not 100% clear whether these are normal and should be allowed or not, so for the moment we'll emit a warning whenever a zero-dimension show rect is seen. For now they only seem to be generated by plane transitions, and these zero-dimension screen items cannot simply be omitted because the 2.1early transitions code requires a fixed number of screen items per step.
When new bitmaps are added and the underlying Common::Array needs to move to expand, this invalidates all pointers to bitmaps, which makes it basically impossible to use the bitmap segment since you never know if a reference is going to be invalidated due to an array move. To solve this, BitmapTable is changed to hold pointers to SciBitmaps that are allocated separately on the heap instead, so when those bitmaps are looked up, the resulting pointers are valid for the lifetime of the bitmap, instead of the lifetime of the Common::Array used internally by BitmapTable.
csnover
force-pushed
the
sci32-transitions
branch
from
a23ec56
to
c602115
Compare
|
Thanks for the changes :) Could you please implement the bitmap sync code for saved games, so that this whole feature is fully implemented before merging it into master? Other than that missing functionality, it looks great :) |
csnover
force-pushed
the
sci32-transitions
branch
from
73d3e8d
to
8be5c99
Compare
csnover
force-pushed
the
sci32-transitions
branch
from
8be5c99
to
f72a13c
Compare
|
Excellent, thanks! :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cleans up the existing transitions code, and implements all other transitions that I could find in SCI32 games:
I feel like there are probably more used transitions than these, but since kSetShowStyle is called indirectly through the Styler game script, it’s a challenge to find all places where it is used. So, for the moment, just like with the magnifier view, any time a game calls to use one of the unknown transition styles, the engine will emit an error message asking the user to tell us what they were doing when that happened, and quit.
This PR also includes a commit that should allow hunk allocations to opt-out of VM garbage collection. This is necessary to avoid GC of the pixel dissolve bitmap, which is never retained by a game script. (The same problem occurs with Robot cels, so this change is needed for Robot, too, and was also an issue for ScrollWindow, which had previously been hooked directly into the GC.)