Project binary injection [old version] #81

KANAjetzt · 2023-01-27T15:54:15Z

Project binary injection

Because we cant set the order of autoloads via override.cfg we have to use external tooling ( GodotPckTool ) to inject the modified project settings in to the games .pck file.

Relevant feature proposal:
godotengine/godot-proposals#6137

OS.execute() - GodotPckTool

# Add modified binary to the pck
func inject_project_binary() -> void:
	var output_add_project_binary := []
	var _exit_code_add_project_binary := OS.execute(path.pck_tool, ["--pack", path.pck, "--action", "add", "--file", path.project_binary, "--remove-prefix", path.mod_loader_dir], true, output_add_project_binary)
	modloaderutils.log_debug_json_print("Adding custom project.binaray to res://", output_add_project_binary, LOG_NAME)

This is the main part of this PR.
Executing the GodotPckTool, adding the before created project.binary to the main .pck.

	# save the current project settings to a new project.binary
	ProjectSettings.save_custom(exe_path + "addons/mod_loader/project.binary")

New cli arguments

--silent

Executes the setup with no user interaction.
Restarts the game immediately after the setup, without showing the countdown

--setup-create-override-cfg

Forces the setup to create the override.cfg in the game base directory.
Skips the project.binary injection.
The setup checks if there is a exe_name.pck file in the game base directory and decides based on this if the injection can happen. So this argument is only required if this behavior is not wanted.

--exe-name="name"

Override the exe_name.

--pck-name="name"

Override the pck_name.

Things this PR not covers

Installation on OSX
Installation on Linux
Installation for embedded .pck files

closes #34

addons/mod_loader/mod_loader_setup.gd

addons/mod_loader/godotpcktool/LICENSE

addons/mod_loader/mod_loader_setup.gd

addons/mod_loader/mod_loader.gd

addons/mod_loader/mod_loader_setup.gd

ithinkandicode · 2023-01-27T20:48:09Z

addons/mod_loader/mod_loader_setup.gd

 		ProjectSettings.set_setting(settings.IS_LOADER_SETUP_APPLIED, true)
 		ProjectSettings.save_custom(modloaderutils.get_override_path())
+
+		# run the game again to apply the changed project settings
+		var _exit_code_game_start = OS.execute(exe_path + "Brotato.exe", ["--script", "addons/mod_loader/mod_loader_setup.gd", '--pck-name="Brotato.pck"', "--log-debug"], false)


@Qubus0 is correct, we can't hardcode the EXE name.

Regarding the starting of the game only working on Windows, I think that's fine for the MVP if this only happens when using the --silent CLI arg (see my other review comment here), because we can just say "silent installation is currently only supported on Windows" -- then we can expand that later. This would let us, for example, allow silent installations on Thunderstore by default, if we choose to do that.

As long as it's an opt-in to run platform-specific code, we won't be preventing users who aren't on that platform from installing/running ModLoader, and that's fine for the MVP. Just be sure to keep the docs updated, perhaps with a dedicated "Platform Specifics" page to maximise its visability ;)

I fixed the hard coded exe name ( not sure how I missed that one in the first place 👀 )

About the --silent
Currently I have it like this:

func restart_game() -> void: # run the game again to apply the changed project settings var _exit_code_game_start = OS.execute(path.exe, ["--script", path.mod_loader_dir + "mod_loader_setup.gd", "--log-debug"], false) # quit the current execution quit()

So the game is always executed before quitting the first instance.
What I read here is that the default should be OS.alert() and leave the restart to the user - right?

In that case I will add --unattended to show the UI and auto restart.

So we have:

no arg -> user handles the restart after OS.alert() message

--unattended user gets notified by restart timer - setup handles "restart"

--silent no restart notification - setup immediately "restarts" the game

From the latest discord chat with Mythic:

--only-setup no restart - setup just quits the current game instance

"restart" in quotes because it's not really a restart, we have to execute the second game instance before we quit the first one.

Sounds good, as long as CLI args are documented I'm happy :)

addons/mod_loader/mod_loader_setup.gd

ithinkandicode · 2023-01-27T23:20:31Z

@KANAjetzt We've been talking on Discord about offering a CLI arg that lets you only save the override.cfg file. Could you include support for this in this PR please? The current use case is Dome Keeper, which has an embedded PCK, so we can't patch it with the methods currently applied in this PR.

Eg. the CLI arg could be --setup-create-override-cfg

Notes

Dome Keeper users could use GodotPCKExplorer to extract the PCK, but that'll need to be done after every game update, and can't be handled automatically, so this isn't really viable if the user is using Thunderstore. Granted, override.cfg has the major limitation of not being able to put ModLoader at the top of the autoloads list (and Dome Keeper does indeed have many autoloads of its own), but @Qubus0 has suggested that this is a reasonable trade-off considering how impractical the alternative would be (ie. repeatedly using GodotPCKExplorer), and that their knowledge of the current scope of Dome Keeper mods suggests that modifying DK's autoloads may not be necessary.

I also think that this is a good thing to support anyway, and will keep the code that generates override.cfg in place for if/when Godot implements Ste's proposal to allow changing the autoloads order.

Note: I also mentioned this approach briefly in this review comment

ithinkandicode · 2023-01-29T05:30:50Z

Once this PR is finished and merged, we can look into extending the features here for #83

KANAjetzt · 2023-01-30T09:12:26Z

@ithinkandicode
--setup-create-override-cfg is implemented , but for some reason the override.cfg is still created.

otDan · 2023-01-31T17:09:38Z

what's missing still @KANAjetzt?

Qubus0 · 2023-01-31T20:35:44Z

addons/mod_loader/mod_loader_setup.gd

+		else:
+			restart_timer.start(4)
+
+		ProjectSettings.set_setting(settings.IS_LOADER_SETUP_APPLIED, true)


this set_setting (and the one in L75) has no effect if it isn't saved to anything.

otDan · 2023-02-01T20:03:54Z

addons/mod_loader/mod_loader_setup.gd

+	modloaderutils.log_debug_json_print("file_name: ", file_name, LOG_NAME)
+
+
+func setup_ui() -> void:


are we sure this will always be on top of everything?
if for example some autoload has stuff that visualizes a game loading bar or something along those lines

ithinkandicode · 2023-02-01T20:19:24Z

addons/mod_loader/mod_loader_setup.gd

+
+		var icon := Image.new()
+		var _error_load_icon := icon.load(ProjectSettings.get_setting("application/config/icon"))
+		OS.set_icon(icon)


Could you add a brief comment to the code here please, stating why this is needed?

ithinkandicode · 2023-02-01T20:20:42Z

addons/mod_loader/mod_loader_setup.gd

+	if not modloaderutils.dir_exists(path.game_base_dir + "mods"):
+		var dir = Directory.new()
+		dir.make_dir(path.game_base_dir + "mods")
+		modloaderutils.log_debug("created mods folder" , LOG_NAME)


This is great but would it not be better in its own PR, dedicated to closing #34?

I've closed issue #34 -- TL;DR is that I don't think this is needed because some games might use Steam Workshop etc

ithinkandicode · 2023-02-01T20:30:42Z

addons/mod_loader/mod_loader_setup.gd

+	# In that case we add a override.cfg file to the base game dir.
+	else:
+		modloaderutils.log_debug("using the override.cfg file", LOG_NAME)
+		create_override_cfg()


This will trigger if either is_setup_create_override_cfg is true, or the PCK file doesn't exist. Is this intentional? Or shouldn't it only trigger if is_setup_create_override_cfg is true?

It's a nice idea to have things happen automatically, but I think it's generally better for things to happen predictably -- eg. if a CLI arg exists to perform an action, that action should only be performed if you use that CLI arg.

This would also let you perform both actions, if (for whatever reason) you might want to. Atm this code only lets you create the override.cfg if the PCK file doesn't exist, but I don't think we need to add this extra level of conditional complexity. Best to just let users interact with the API without such hidden conditions.

I might be wrong here so I'm open to discussion

ithinkandicode · 2023-02-01T20:42:36Z

addons/mod_loader/mod_loader_setup.gd

+	modloaderutils.log_debug_json_print("file_name: ", file_name, LOG_NAME)
+
+
+func setup_ui() -> void:


Maybe make the GUI stuff optional via a CLI arg?

Tbh though I think this GUI feature would be better in a separate subsequent PR, as this PR is focused on patching the binary. Best to keep things small and contained IMO, makes it easier to review, keeps discussions focused, and gets stuff merged faster

ithinkandicode · 2023-02-01T20:46:33Z

@KANAjetzt Don't forget to fix the merge conflict! ;)

otDan · 2023-02-01T20:47:03Z

poor @KANAjetzt hahaha, we believe in you!

ithinkandicode · 2023-02-04T00:50:15Z

This was closed because it's doing several things, so KANA is going to split it up into smaller chunks, eg:

Project binary injection #89

KANAjetzt added 4 commits January 26, 2023 21:08

✅added project.binary injection with gdpck-tool

7e01c48

✔ no longer injecting mod loader scripts

3befddc

✔ added autoload order debug log

cca3415

✔ added game execution before quitting setup

27c7d4c

KANAjetzt added the enhancement New feature or request label Jan 27, 2023

KANAjetzt requested a review from a team January 27, 2023 15:54

Qubus0 reviewed Jan 27, 2023

View reviewed changes

addons/mod_loader/mod_loader_setup.gd Outdated Show resolved Hide resolved

ithinkandicode reviewed Jan 27, 2023

View reviewed changes

addons/mod_loader/mod_loader_setup.gd Outdated Show resolved Hide resolved

ithinkandicode mentioned this pull request Jan 27, 2023

Support embedded PCKs #83

Closed

KANAjetzt added 5 commits January 28, 2023 21:37

🧹 clean up path logging

ee51c3c

🔨 add vendor folder

1eb0520

✔ added --exe-name cli arg

65fa5c9

🔨 removed the backup process

f5c2bc6

🧹 remove the autoload order logging

20b0a90

KANAjetzt added 12 commits January 29, 2023 14:12

✔ added restart notification

7a8c627

🧹 move project.binary injection to dedicated func

fe936cf

🧹 moved project binary creation in dedicated func

4de915d

🔨 fixed linting errors and typos

b1d993c

✔ added --silent argument

614691d

🛠 WIP: --setup-create-override-cfg cli arg

8cc7596

🔨 added missing empty lines

96ab3fa

✔ create mods dir if not there

4c6901a

🔨 fix icon getting lost when using --script

71c3b4f

🧹 moved unnecessary if statements

453496c

👀 accidentally pushed the project.binary file

b9fc0bc

✔ added project.binary file cleanup

5b8f46c

Qubus0 reviewed Jan 31, 2023

View reviewed changes

otDan reviewed Feb 1, 2023

View reviewed changes

ithinkandicode reviewed Feb 1, 2023

View reviewed changes

ithinkandicode mentioned this pull request Feb 2, 2023

automatically create mods and mods-unpacked directories #34

Closed

KANAjetzt added 9 commits February 3, 2023 22:04

✔ added --unattended cmd arg

c81a828

🔨 only setup UI if --unattended

1e120e7

✔ added cli arg --only-setup

45fa127

🔨 added comment why OS.set_icon() is required

b327488

🧹 removed auto creation of mods dir -> see #34

1906ca5

🔨 fixed comments on restart handling

90223a1

👀 fixed spacing

d756abb

✔ added ModLoader autoload index check

9354b10

🧹 added modded_start() func

e636796

KANAjetzt closed this Feb 3, 2023

KANAjetzt deleted the project_binary_injection branch February 3, 2023 22:48

ithinkandicode changed the title ~~Project binary injection~~ Project binary injection [old version] Feb 4, 2023

ithinkandicode mentioned this pull request Feb 5, 2023

Assert modloader is first autoload #96

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Project binary injection [old version] #81

Project binary injection [old version] #81

KANAjetzt commented Jan 27, 2023 •

edited

Loading

ithinkandicode Jan 27, 2023 •

edited

Loading

KANAjetzt Jan 30, 2023 •

edited

Loading

ithinkandicode Feb 1, 2023

ithinkandicode commented Jan 27, 2023 •

edited

Loading

ithinkandicode commented Jan 29, 2023

KANAjetzt commented Jan 30, 2023 •

edited

Loading

otDan commented Jan 31, 2023

Qubus0 Jan 31, 2023

otDan Feb 1, 2023

ithinkandicode Feb 1, 2023

ithinkandicode Feb 1, 2023 •

edited

Loading

ithinkandicode Feb 2, 2023 •

edited

Loading

ithinkandicode Feb 1, 2023

ithinkandicode Feb 1, 2023

ithinkandicode commented Feb 1, 2023

otDan commented Feb 1, 2023

ithinkandicode commented Feb 4, 2023

		modloaderutils.log_debug_json_print("file_name: ", file_name, LOG_NAME)


		func setup_ui() -> void:

Project binary injection [old version] #81

Project binary injection [old version] #81

Conversation

KANAjetzt commented Jan 27, 2023 • edited Loading

Project binary injection

OS.execute() - GodotPckTool

New cli arguments

--silent

--setup-create-override-cfg

--exe-name="name"

--pck-name="name"

Things this PR not covers

ithinkandicode Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

KANAjetzt Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

ithinkandicode Feb 1, 2023

Choose a reason for hiding this comment

ithinkandicode commented Jan 27, 2023 • edited Loading

ithinkandicode commented Jan 29, 2023

KANAjetzt commented Jan 30, 2023 • edited Loading

otDan commented Jan 31, 2023

Qubus0 Jan 31, 2023

Choose a reason for hiding this comment

otDan Feb 1, 2023

Choose a reason for hiding this comment

ithinkandicode Feb 1, 2023

Choose a reason for hiding this comment

ithinkandicode Feb 1, 2023 • edited Loading

Choose a reason for hiding this comment

ithinkandicode Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

ithinkandicode Feb 1, 2023

Choose a reason for hiding this comment

ithinkandicode Feb 1, 2023

Choose a reason for hiding this comment

ithinkandicode commented Feb 1, 2023

otDan commented Feb 1, 2023

ithinkandicode commented Feb 4, 2023

KANAjetzt commented Jan 27, 2023 •

edited

Loading

ithinkandicode Jan 27, 2023 •

edited

Loading

KANAjetzt Jan 30, 2023 •

edited

Loading

ithinkandicode commented Jan 27, 2023 •

edited

Loading

KANAjetzt commented Jan 30, 2023 •

edited

Loading

ithinkandicode Feb 1, 2023 •

edited

Loading

ithinkandicode Feb 2, 2023 •

edited

Loading