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 godot.ico to uncompressed/fixed size #57850

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

pkowal1982
Copy link
Contributor

I think it should go into 3.x first but as the workflow says that master should precede everything, I'm creating PR to master.

Change is small and unobtrusive but fixes some Windows exporting issues.

Using windows icon file which is uncompressed (has fixed size) allows for easy icon replacement even in executables with pck embedded. Of course replacement icon should have the same size.
To create such icon you'll need a tool which I've happily already created:

➡️ https://github.com/pkowal1982/godoticon ⬅️

Don't worry, no external tools needed.

How to use it:

  1. Prepare your icon (or icon versions in proper resolutions) in graphics editor
  2. Convert created graphics into icon with provided script: https://github.com/pkowal1982/godoticon/blob/master/CreateIcon.gd
  3. Export your game with proper windows template (template should use fixed size icon)
  4. Replace default icon with the one created in the second step using script: https://github.com/pkowal1982/godoticon/blob/master/ReplaceIcon.gd

Pros:

  • Works with embedded pck (no need to recompile template with custom icon, official one should work)
  • No need to download/use magick
  • No need to download/use rcedit which becomes real pain in the ass if you're using Linux. If you still like to use rcedit you can, it should work as before but also for executables with embedded pck
  • Just run these Godot scripts from console, no external tools needed, it's system independent

Cons:

  • uncompressed Windows template/executable is bigger by 243 KB

@pkowal1982 pkowal1982 requested a review from a team as a code owner February 9, 2022 12:22
@Chaosus Chaosus added this to the 4.0 milestone Feb 9, 2022
@Calinou
Copy link
Member

Calinou commented Feb 9, 2022

This effectively reverts #9285, but I guess it's fine as ~250 KB isn't so much for a Windows binary by 2022 standards. This approach will also make it easier for the Godot editor to eventually support replacing icons on export without relying on any external tools.

I wonder if we could use the same hex-editing approach for replacing other executable metadata, such as the version and description. We could have a default description padded with spaces to allow for fairly long descriptions.

@pkowal1982
Copy link
Contributor Author

Padding description with spaces should work. Extending the replacement script should be easy basing on what's already implemented and it can be done independently of this PR as I assume there are some decisions to be made about version "fixed" format which can take time.

@aaronfranke
Copy link
Member

Is there a reason that we can't make it easy to change the icon during export instead of using icon replacement tools after export?

@bruvzg
Copy link
Member

bruvzg commented Feb 9, 2022

Works with embedded pck (no need to recompile template with custom icon, official one should work)

It's easy to fix by running rcedit before embedding PCK, see #56093. But adding an extra 200 KB isn't an issue. And it won't break anything, so it should be fine.

@pkowal1982
Copy link
Contributor Author

@aaronfranke The reason to use icon replacement tool is, that this tool already exists and all it needs is merging this "fixed" icon to master and 3.x branches. If someone will rewrite logic from the script to the editor itself it won't need external tools anymore. But it will still need this fixed size approach.
@bruvzg rcedit is fine if you're using Windows for development. Running rcedit on Linux it's not so easy. :)

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

I think the pros outweigh the cons here, so we can give this a try in master at least (and then consider it for 3.x).

Compressed binary size (e.g. within a ZIP) shouldn't be affected much by this change, so download times for editor and export template binaries should remain almost identical.

@akien-mga akien-mga merged commit 70eb95c into godotengine:master Mar 18, 2022
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Mar 18, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@hsandt
Copy link
Contributor

hsandt commented Apr 27, 2023

Sorry to comment on an old PR, but that sounds like a nice alternative to rcedit, shouldn't it be more promoted and added to the doc (https://docs.godotengine.org/en/stable/tutorials/export/changing_application_icon_for_windows.html)? Is it an issue because it is third-party code and cannot not fully endorsed by the Godot team?

Also, to avoid the leftover warning in the export popup:

The rcedit tool must be configured in the Editor Settings (Export > Windows > rcedit) to change the icon or app information data.

and on build:

WARNING: Resources Modification: Could not start rcedit executable. Configure rcedit path in the Editor Settings (Export > Windows > rcedit), or disable "Application > Modify Resources" in the export preset.

It seems we must disable "Application > Modify Resources" so Godot doesn't try to access rcedit. Can you confirm that it won't have other side effects, and that the only "modified resource" is the icon?

Finally, we could add the hint about disabling "Application > Modify Resources" to the first warning, in the export popup, so the user doesn't have to wait for an actual export to see the hint in the second warning.

If you're okay with this, I'll open respective issues in godot-docs repo and godot-proposals.

@pkowal1982
Copy link
Contributor Author

@hsandt You should probably look at #75950

hsandt added a commit to hsandt/godot-project-bootstrap that referenced this pull request Apr 28, 2023
…com/pkowal1982/godoticon (latest change 2023-02-17)

and add 6 icon formats.

Follow instructions in godotengine/godot#57850 to replace Windows icon after export.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants