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

Added support for replacing game scripts #179

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

zykurv
Copy link

@zykurv zykurv commented Mar 26, 2024

Script replacements are configured with a new "replaceScripts" option in mkxp.json. Because you can have multiple scripts with the same names you can also specify using the script index instead.

@Splendide-Imaginarius
Copy link

What's the use case here? Is there a reason why preload/postload scripts would be less suitable? (Not saying I'm against this feature, just trying to understand the reasoning.)

@zykurv
Copy link
Author

zykurv commented Apr 2, 2024

@Splendide-Imaginarius scripts aren't necessarily pure, they can have side effects... Which means a script that is causing a problem during load cannot be patched postload, because at that point the script side effects were already executed. And I don't think there'd be anything preload that can be done to avoid the faulty script from executing either.

There's also a problem I found in one case, where the inheritance of a class is changed after it was originally defined. This used to work in the Ruby version that RMVX uses, but causes an error with the version being used by mkxp-z. Because of that the game won't work, you need to replace the script to ensure the inheritance is consistent for all the class declarations.

@WaywardHeart
Copy link

And I don't think there'd be anything preload that can be done to avoid the faulty script from executing either.

@zykurv Preload scripts can access and modify the game scripts using the $RGSS_SCRIPTS global variable.

ensure the inheritance is consistent for all the class declarations

If I understand what you mean then that shouldn't actually be an issue for us. Our fork of ruby has a patch that removes that error.

@WaywardHeart
Copy link

WaywardHeart commented Apr 2, 2024

@Splendide-Imaginarius I just did some testing, and in Ruby 1.8 you could actually change the super class by doing that - which we're not replicating. Do we want to change that? Ace games shouldn't be doing that at all, so we probably wouldn't be introducing any bugs by adding that functionality.

Edit: Scratch that, it doesn't change the super class, it generates a new class and replaces the constant.

@WaywardHeart
Copy link

Alright, I found the problem. It turns out Struma had tried to fix it, but had only removed the error in one of the code paths. I also included a fix for a syntax error I've ran into before. Thanks for bringing that to our attention @zykurv.

@zykurv
Copy link
Author

zykurv commented Apr 4, 2024

@WaywardHeart oops sorry, I misremembered the issue. Yeah the issue was that "super()" was calling the functions from the wrong class after changing the inheritance. Thanks for investigating ^_^

@zykurv
Copy link
Author

zykurv commented Apr 4, 2024

@zykurv Preload scripts can access and modify the game scripts using the $RGSS_SCRIPTS global variable.

Sorry, I didn't knew that... after some testing I was able to do the script replacement with the preload script below. Thanks for the info, I guess this PR is unneeded then.

$RGSS_REPLACE = {
    "GameScriptFoo" => "scripts/Foo.rb",
    "123" => "scripts/Bar.rb",
}

for index in 0...$RGSS_SCRIPTS.size
    script_info = $RGSS_SCRIPTS[index]
    script_name = script_info[1]

    replace_file = nil
    replace_file = $RGSS_REPLACE[index.to_s] if $RGSS_REPLACE[index.to_s] != nil
    replace_file = $RGSS_REPLACE[script_name] if $RGSS_REPLACE[script_name] != nil
    next if replace_file == nil
    
    File.open(replace_file, "r") do |file|
        script_info[3] = file.read
    end
end

@Splendide-Imaginarius
Copy link

@zykurv Would you be willing to submit a PR that adds that preload script to the scripts/preload folder? (Also I didn't know it was accessible that way either, thanks @WaywardHeart for the info!)

@zykurv
Copy link
Author

zykurv commented Apr 4, 2024

@Splendide-Imaginarius PR opened here: #180

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