Skip to content

Various Lingo fixes and error handling changes - #94

Open
zdimension wants to merge 16 commits into
igorlira:mainfrom
zdimension:pr3small
Open

Various Lingo fixes and error handling changes#94
zdimension wants to merge 16 commits into
igorlira:mainfrom
zdimension:pr3small

Conversation

@zdimension

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread vm-rust/src/player/script.rs Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's hard to tell what's being changed in this file because the diff is affecting the entire file. I suspect this might be an issue with the line ending that your git client is configured to use. The file uses LF line endings, but this PR converts the entire file to CRLF. Can you run the following command to ensure your git uses LF line endings, and the re-commit this file?

git config --global core.autocrlf true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, yeah there must have been some line ending weirdness, I'll fix that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Comment on lines -83 to +89
if let Some(member) = member {
Ok(player.alloc_datum(Datum::CastMember(member.to_owned())))
} else {
Ok(player.alloc_datum(Datum::CastMember(INVALID_CAST_MEMBER_REF)))
}

Ok(player.alloc_datum(match member {
Some(r) => Datum::CastMember(r),
None => Datum::Void
}))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Whether member("invalid") returns INVALID_CAST_MEMBER_REF or Void is dependent on the value of _player.scriptExecutionStyle. There are two possible values (9 or 10) and I believe it is automatically set based on the version of the dir/dcr file (scriptExecutionStyle = 9 for < 1000, and scriptExecutionStyle = 10 for >= 1000)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I think to faithfully reproduce Director's behavior we should introduce a scriptExecutionStyle property that, same as Director, is automatically set depending on the version we're emulating, and that can be changed from Lingo. If that's okay for you, I can add it to the PR!

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Absolutely! Let's add it this PR so that it doesn't break existing movies that depend on the INVALID_CAST_MEMBER_REF behavior

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! also merged main onto my branch and resolved the conflicts. PR shows many files changed but this is because of the merge commit. (It was way faster than to rebase)

@igorlira igorlira Apr 5, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This file also has the newline issue, the diff is affecting the entire file. Could you convert CRLF to LF?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

uh, weird. I can see that when I look at the PR's changes, but not by looking at each individual commit... I'm gonna try from the CLI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, the CRLF changes seem to come from this commit 5cf9b36 which is not mine?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants