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

DIRECTOR: Fix sprite position setting #5074

Merged
merged 2 commits into from Jun 10, 2023

Conversation

hari01584
Copy link
Contributor

Self explanatory, do not merge, WIP.

In original engine it is possible to set non-puppted sprite properties
like position, backcolor etc and then call updatestage to update the
screen according to property. This is undocumented behaviour
which is not present depicted in any books however with rigorous testing
it is found to be true. This change only persist for the current frame
and once the playback head is moving the change is lost.

`ATD\HD\bdDREAMA.DXR` of 'totaldistortion' used some of this quirk to
display moving bullets.
Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

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

The code is only partially correct. Other changes are hacky and I do not see why you're adding them

@@ -62,6 +62,8 @@ Sprite::Sprite(Frame *frame) {
_moveable = false;
_editable = false;
_puppet = false;
if (g_director->getVersion() >= 600)
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect. You already setting this flag in the individual places where we change properties.

With this code, you blindly assign autopuppet to any non-puppet sprite.

@@ -564,6 +564,7 @@ void Score::renderFrame(uint16 frameId, RenderMode mode) {
setLastPalette(frameId);
renderSprites(frameId, mode);
_window->render();
resetNonPuppetSprites(frameId);
Copy link
Member

Choose a reason for hiding this comment

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

Please explain why are you doing this? I thought, that this reset is happening in Channel::setClean()

		if (_sprite->_puppet || (!nextSprite->isQDShape() && partial)) {
			// Updating scripts, etc. does not require a full re-render
			_sprite->_scriptId = nextSprite->_scriptId;
		} else {
			previousCastId = _sprite->_castId;
			replaceSprite(nextSprite);
		}

e.g. we are not touching puppetized sprites

@@ -537,24 +555,47 @@ void Channel::replaceSprite(Sprite *nextSprite) {
}
}

void Channel::setNonChangedSprite(Sprite *nextSprite) {
Copy link
Member

Choose a reason for hiding this comment

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

What is the prurpose of it? Looks like a dirty hack or no reason. You should not check the positions like this.

[Based on Director in a Nutshell, page 15]
The following properties of a sprite are auto-puppeted whenever
the property is set: backColor, blend, editable, foreColor, height,
ink, loc, locH, locV, member, moveable, rect and width. Auto-puppeting
of individual properties has no effect on the puppet of sprite property

['Lingo Dictionary' 601 Fixes]
Sprites now have a duration as specified in the (new) score view.
When lingo modifies any of the sprite properties that property
will be auto-puppeted, i.e. the value set will stick for the
duration of that sprite despite any values recorded in the score.
This can cause existing lingo which relied on setting the property
of a sprite without the use of 'puppet' to behave differently.
Calling 'puppetSprite spriteNumber, FALSE' will also clear any
currently auto-puppeted properties.
@hari01584 hari01584 marked this pull request as ready for review June 10, 2023 14:15
@sev-
Copy link
Member

sev- commented Jun 10, 2023

Thank you!

@sev- sev- merged commit 5a5e205 into scummvm:master Jun 10, 2023
8 checks passed
@lephilousophe lephilousophe added the GSoC Part of a Google Summer of Code project label Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Part of a Google Summer of Code project
Projects
None yet
3 participants