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

RPA checks + scripts.rpa handling #10212

Open
wants to merge 4 commits into
base: content
Choose a base branch
from

Conversation

ThePotatoGuy
Copy link
Member

@ThePotatoGuy ThePotatoGuy commented Feb 25, 2024

Changes

  • Re-added splash RPA check
  • added override to skip loading init code for script-poemgame
    • this was done by hooking into a spot between where early scripts are loaded and init scripts are loaded and removing the AST nodes for script-poemgame from renpy.game.script.initcode so it doesnt load later. (see override function comment for more info)
  • updated CI so it doesnt delete scripts.rpa anymore

Testing

loading with no rpas

  1. remove the DDLC rpas from the folder
  2. launch the mod
  3. verify the game shows the missing archives error

loading with scripts.rpa

  1. Add the DDLC rpas (including scripts.rpa)
  2. launch the mod
  3. verify the game launches / no crashes from scripts.rpa

@ThePotatoGuy ThePotatoGuy added awaiting testing code needs to be tested awaiting code review someone needs to check for syntax/logic/indentation errors labels Feb 25, 2024
@ThePotatoGuy ThePotatoGuy added this to the 0.13.0 milestone Feb 25, 2024
Copy link
Member

@Booplicate Booplicate left a comment

Choose a reason for hiding this comment

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

@ThePotatoGuy I am a terrible reviewer because this was done almost a year ago, but could you remind me why we need this? We should build scripts.rpa ourselves and users will overwrite the files (or it will do our installer). We don't use anything from the original archive anymore, so it should work. This is much simpler than relying on py2 code and doing this handling and maintaining extra overrides.

This is also legal because we still check for

  1. the scripts.rpa being present when the game starts
  2. the scripts.rpa being present when the installer runs
  3. other rpa from the original game when the game starts

I believe we should just uncomment the check and stop deleting rpa in our CI

@multimokia am I wrong?

@multimokia
Copy link
Member

I remember I think deleting was my call due to removing the dependency and it causing issues.

I agree, if we can simplify the process by replacing the files, that may be a better call.
If anything, we can also disable the old rpa for those who wish to uninstall? Not mandatory, but if necessary is doable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting code review someone needs to check for syntax/logic/indentation errors awaiting testing code needs to be tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants