Display project settings splash color on web export

[elpozewaunig]

This is a minor improvement to the new web export splash screen, which now displays the correct splash color specified in the project settings as the background, as was mentioned in #91852.

I've been a fan of Godot for quite some time now and have been using it for hobby projects, as well as convincing friends to try it. I don't have much experience with the inner workings of the engine, or C++ for that matter but I got very excited to find that I can help out with small additions too ;). My changes have been tested locally and work as expected but since this is my very first contribution to Godot, I'd appreciate a thorough check and I hope that everything is alright!

• Bugsquad edit, closes: [4.3-stable]Changing the Boot Splash background color does not reflect in the web export #96874

[Calinou]

Looks good to me.

One concern however is that there's compatibility breakage for custom HTML shells, since $GODOT_SPLASH was renamed to $GODOT_SPLASH_IMAGE. It's probably not a dealbreaker and is unavoidable here, but it should be highlighted in the release notes and the upgrade guide.

[timothyqiu]

We can keep $GODOT_SPLASH as-is, or provide $GODOT_SPLASH along with $GODOT_SPLASH_IMAGE for compatibility.

[elpozewaunig]

Both can work, but keeping $GODOT_SPLASH the way it is requires a specific order in the replaces HashMap for it not to break the $GODOT_SPLASH_COLOR placeholder. I think it is also more intuitive to name them more similarly to the project settings they apply, so if we want to keep the compatibility, providing it alongside $GODOT_SPLASH_IMAGE might be the better way to go.

[elpozewaunig]

Has there been any decision yet as to how we should proceed regarding compatibility? I'm happy to make any changes needed.

[adamscott]

Even if I like up-to-date code, it doesn't warrant breaking compatibility, really.

Both can work, but keeping $GODOT_SPLASH the way it is requires a specific order in the replaces HashMap for it not to break the $GODOT_SPLASH_COLOR placeholder.

I don't understand how it could break.

Has there been any decision yet as to how we should proceed regarding compatibility?

Sorry for the delay! Don't hesitate to join the developers' chat #Web channel in order to discuss topics.

[elpozewaunig]

I don't understand how it could break.

If $GODOT_SPLASH is replaced before $GODOT_SPLASH_COLOR, it changes the "$GODOT_SPLASH_COLOR" string in the CSS to e.g. "splashimage.png_COLOR", breaking the value and preventing the correct replacement (since the placeholder string doesn't exist in the CSS anymore). I just realised my initial assessment was likely wrong too (though I can't test it currently) - changing the order in the code wouldn't reliably work either, since we're dealing with a HashMap. If we want to revert back to the old name, we would either have to rename $GODOT_SPLASH_COLOR so that $GODOT_SPLASH is no longer a prefix, or change the implementation of the replacement.

Even if I like up-to-date code, it doesn't warrant breaking compatibility, really.

I agree! I was under the impression that strict compatibility preservation is only done for the API, not for the export system, which is why I didn't think of this problem.

Sorry for the delay! Don't hesitate to join the developers' chat #Web channel in order to discuss topics.

Done, much appreciated!

[elpozewaunig]

I just realised my initial assessment was likely wrong too (though I can't test it currently) - changing the order in the code wouldn't reliably work either, since we're dealing with a HashMap.

I was able to test this right now and skimmed over the implementation too, and I believe Godot's HashMap iteration actually does return elements in the order they were added. So the options I currently see are the following:

• Keep the rename, highlight it as a breaking change for custom HTML shells (I'd prefer this, as it follows a clear, understandable naming scheme similar to the project settings)
• Keep the old name, change the $GODOT_SPLASH_COLOR placeholder to something else (maybe $GODOT_COLOR_SPLASH? Seems inconsistent to me though)
• Keep the old name, change the order so that $GODOT_SPLASH is after $GODOT_SPLASH_COLOR
• Keep the old name, rework the replacement logic to not replace a string if there is a non-terminating character after the match

[adamscott]

• Keep the old name, change the order so that $GODOT_SPLASH is after $GODOT_SPLASH_COLOR

Yes. This is the simplest solution.

[elpozewaunig]

Alright, I made the change now! I hope we can eventually move to a variable name that matches the project settings since relying on the order seems a little fragile to me but for now, I think this is good to merge! Let me know if there's anything else I should change and thank you for taking the time to review this PR. :)

[Repiteo]

Please squash your commits into one, see the interactive rebase for details

[elpozewaunig]

@Repiteo Done! Sorry for the inconvenience!

[Repiteo]

Thanks! Congratulations on your first contribution! 🎉