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

Change: Unreachable range for default only switch should target CB_FAILED #328

Merged

Conversation

JGRennison
Copy link
Contributor

The unreachable range for default only switches should target the CB_FAILED group, instead of returning a success value of 0.

This is because the current success value of 0 interacts unfavourably with other unfortunate quirks of the GRF spec and how callbacks are handled.

  • The error group for a deterministic sprite group is the first range group (not the default group), so it is the unreachable group in this case.
  • For historical reasons, unhandled callbacks are routed onto the graphics chain. (This anti-pattern should be removed, but this is outside the scope of this PR).
  • Many key callbacks are unmasked and so always executed, therefore will end up on the graphics chain if not explicitly handled in the NML source. In particular this includes callback 36.
  • Variables such as vehicle variable 61 are fine when called in a graphics context but route to the error group for most callbacks.

The result of this is that a switch which can trigger a variable error in the graphics chain can result in unrelated callbacks inexplicably succeeding with a value of 0. In the case of callback 36 this can lead to all sorts of weird behaviour.
This PR changes this behaviour to the callbacks returning a failure result as expected.

@JGRennison JGRennison force-pushed the change-default-only-switch-range-value branch from 3cf1714 to 9694390 Compare April 29, 2024 17:22
…ILED

Instead of returning a success value of 0
@JGRennison JGRennison force-pushed the change-default-only-switch-range-value branch from 9694390 to 7f7495c Compare July 9, 2024 22:14
@PeterN PeterN merged commit 78850aa into OpenTTD:master Jul 9, 2024
21 checks passed
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