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

Add Olympus support #299

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add Olympus support #299

wants to merge 3 commits into from

Conversation

alexbatalov
Copy link
Owner

A couple of last minute fixes for Olympus.

// arguments instead of tile. Original game (x86) does not distinguish
// between integers and pointers, so one of the tiles is silently ignored
// while calculating rotation. As a workaround this opcode now accepts
// both integers and objects.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the bug should be fixed in the script?) Sounds like a rabbit hole allowing such ambiguity in arguments. If it says tile num, it should be tile num IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or maybe good to implement original game behavior - just ignore, maybe with warning

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, keep the original behavior (only accept int) but write warning in the debug log. This is a really bad idea to try and hack the engine around bugs in scripts :)

@roginvs
Copy link
Contributor

roginvs commented Jul 13, 2023

Can we merge this PR? I would like to play Olympus using this project :)

@phobos2077
Copy link
Contributor

phobos2077 commented Jul 13, 2023

In theory, since we now have additional data type in scripts (object pointer), it allows for flexibility like this. However, I think better to reserve it for new opcodes to keep backwards compatibility (keep original behavior for vanilla opcodes). For opcode arguments that accept integer, you can just reinterpret pointer to integer and let it run, or (better) print error and return some default value. Bot do not interrupt the script flow. (this will solve the original issue, since vanilla behavior will be retained)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants