godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.56k stars 20.28k forks source link

Compressed godot.ico brakes Godot Icon tool #73497

Open pkowal1982 opened 1 year ago

pkowal1982 commented 1 year ago

Godot version

4.0 rc 2

System information

All operating systems

Issue description

@bruvzg This commit brakes compability with Godot Icon tool. https://github.com/godotengine/godot/commit/42c2c02acfac6af5d382310e36a473bf36d18944

As agreed before in this commit: https://github.com/godotengine/godot/commit/d469cfb2abb4074fe38097e43e015ebd69929f28

having a little bigger uncompressed Windows icon is ok when it enables users to replace it without rcedit usage. Godot Icon tool is especially helpful on Linux systems.

Also Godot Icon is mentioned in official docs (at least for 3.x) so it would be nice to leave the uncompressed icon.

Steps to reproduce

No steps needed to reproduce.

Minimal reproduction project

As archive is required I'm attaching empty one. empty.zip

bruvzg commented 1 year ago

Uncompressed icon was causing issues - see https://github.com/godotengine/godot/issues/64073, also we have two executable, and two icons (and in case of the console wrapper, uncompressed icon is lately 3 times larger that executable itself).

Also, note that now Godot supports generating windows icon from the project icon, and will try to fix icon order on the export, so any executable that will rcedit will have compressed icon regardless of the source icon or default icon compression.

pkowal1982 commented 1 year ago

I would assume that the only icon changed by user will be godot.ico not the console wrapper one so this excess file size would be acceptable.

As I understand #64073 was caused by rcedit bug. It's strange to me that we try to fit our software to this buggy behaviour. Remember that some people started to use Godot Icon and they liked it, especially for cross system compatibility and small download size.

If the problem lies in rcedit wrongly sorting images we should state it to the people so they know why their icons are blurred and how to prevent it. If rcedit produces wrong output they can still use icon created with Godot Icon tool? Probably it would mitigate the problem. Does changing icon with rcedit stopped braking executables with embedded pck?

I would restore uncompressed godot.ico and educate people how to prevent icon blurring on their exports.

bruvzg commented 1 year ago

I would assume that the only icon changed by user will be godot.ico not the console wrapper one so this excess file size would be acceptable.

Both icons can be changed by user.

Does changing icon with rcedit stopped braking executables with embedded pck?

Yes, current export process can be used with embedded PCK, rcedit modification and signed executable. See https://github.com/godotengine/godot/pull/56093.

and educate people how to prevent icon blurring on their exports.

Most users won't read docs, and rcedit is basically a standard tool everyone is using, so bug or not we definitely should support it.

As long as sorting orders used by rcedit, Godot export plugin (see https://github.com/godotengine/godot/pull/68828, it's trying to reorder sizes in the user provided icon), default icon are matching (and not causing any issues).

Personally, I won't oppose uncompressed icon as such, but I'm not sure if support for the external tool that is less versatile than rcedit is convincing reason to have it.

pkowal1982 commented 1 year ago

If we invert the order of images in uncompressed godot.ico it will fix the issues users had? Cause this way we can also support Linux users and everyone will be content.

akien-mga commented 1 year ago

Linux can use rcedit just fine with Wine.

pkowal1982 commented 1 year ago

Ok, looks like Wine should work, just this 1 161 MB of used space seems a little overshot to change the icon. :)

bruvzg commented 1 year ago

Ideally, we want to be able to do it without rcedit, but we need to change other stuff, not just icons. What's status of https://github.com/hpvb/ppelib? AFAIK it was created for this purpose, but I'm not sure if it's usable.

akien-mga commented 1 year ago

Ideally, we want to be able to do it without rcedit, but we need to change other stuff, not just icons. What's status of hpvb/ppelib? AFAIK it was created for this purpose, but I'm not sure if it's usable.

It was fairly advanced as I remember, though @hpvb was maybe being a bit perfectionist with it and didn't get to finish before life caught on. It's probably not far from what we'd need in Godot if you want to take a look / poke hp on RocketChat to discuss it further.

hpvb commented 1 year ago

I made a different one here; https://github.com/hpvb/pperesource which is based on ppelib but tries to be less "perfectionist" :P I'll add resource saving in it over the next week or two and then it can be integrated.

I kind of forgot about this to be entirely honest.

pkowal1982 commented 1 year ago

@hpvb Any news on the topic? Maybe I can help with your library?

keithrdavis commented 1 year ago

I am on a Mac and having a plugin or native support for adding/changing the icon for Windows would be most helpful.

pkowal1982 commented 1 year ago

I've created PR with POC how we could remove rcedit completely.