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: Shortcut varaction chains for callbacks and triggers. #9289

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PeterN
Copy link
Member

@PeterN PeterN commented May 22, 2021

Motivation / Problem

By design, an unhandled callback often ends up following the default graphics chain. Depending on the variables accessed this could potentially be expensive.

Description

If a varaction chain does contain any callback results, do not continue evaluating the chain. This possibly avoids processing complex graphical chains which can never end in a callback result.

Limitations

This is not really benchmarked and is hypothetical at this point. It's not impossible that the additional test causes more work over all.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@frosch123
Copy link
Member

This looks like it need simliar checks as NML's switch optimisation.

  • Rerandomisation is already part of the diff.
  • Writing persistent storage is missing.

@PeterN
Copy link
Member Author

PeterN commented Jun 13, 2021

Added what I think is the case for writing to persistent storage. Not really familiar with that system tbh.

@JGRennison
Copy link
Contributor

Procedure calls (variable 0x7E, DeterministicSpriteGroupAdjust::subroutine) aren't handled. If a deterministic group adjust references a procedure, DSGA_OP_STOP operations (but not any form of callback result) recursively reachable through the procedure group should also set has_cb_result on the referencing group.

A deterministic group with calculated_result set to true is equivalent to a callback result, so should also set has_cb_result.

Less importantly, GRFs will sometimes use groups which branch on variable 0xC, returning a callback result if 0 and some other non-callback group type for non-zero values. This is to implement "fail no matter if this is a callback or not". (This is CB_FAILED in NML). This will generate false positives in has_cb_result when used in the default graphics chain (many GRFs do this).

@PeterN
Copy link
Member Author

PeterN commented Apr 24, 2023

This is now changed to also shortcut sprite group chains during trigger rerandomization if no rerandomization will occur.

This suffers from potential premature optimization. I've not got any performance figures to show that anything needs improving, or that this improves it.

@PeterN PeterN changed the title Change: Shortcut varaction chains for callbacks. Change: Shortcut varaction chains for callbacks and triggers. Apr 24, 2023
Depending on context, if a sprite group chain does contain any callback
results or trigger rerandomization, do not continue evaluating the
chain. This possibly avoids processing complex graphical chains which
can never end in a callback result (or rerandomization).
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.

None yet

3 participants