rooklift / nibbler

Chess analysis GUI for UCI engines, with extra features for Leela (Lc0) in particular.
GNU General Public License v3.0
572 stars 71 forks source link

Replace Linux app icon with svg variant #214

Closed ciriousjoker closed 1 year ago

ciriousjoker commented 1 year ago

In 2020 I've designed an icon for Nibbler https://github.com/rooklift/nibbler/issues/47#issuecomment-657897858, which was never merged.

Now, this icon has been merged for Linux: https://github.com/rooklift/nibbler/pull/210

However, that PR was using the .png variant I provided in 2020. Freedesktop apparently mandates .svg, so I dug up the sources for the icon and exported it as .svg.

However, I also improved it a little (at least in my opinion), by adding a fisheye effect & a gradient to the magnifying glass.

Here are all the sources & exported svgs. AppIcon_sources.zip

I think the updated one looks better, which is why I've used it in this PR, but let me know what you think.


Here's a comparison:

original updated
AppIcon_original AppIcon_updated
ciriousjoker commented 1 year ago

@cyqsimon Your turn ;)

tyvm!

rooklift commented 1 year ago

Thanks, I'll leave this until we merge in the reorganisation of the files - which I think is waiting for an adjusted install.sh script - @cyqsimon do I have that right?

cyqsimon commented 1 year ago

Freedesktop apparently mandates .svg

What I meant to say is that FreeDesktop mandates that svg be supported by desktop environments, alongside png. So technically, your old png icon works just fine =). But using vector graphics is certainly an improvement and more future-proof.

which I think is waiting for an adjusted install.sh script

Correct, I'm working on that.

cyqsimon commented 1 year ago

@ciriousjoker can you please make this change on the reorganise branch instead? We are both working on that branch right now, and there's some major moving-files-around. Sorry for the trouble.

ciriousjoker commented 1 year ago

I can just update this PR once the other one is merged, do u agree?

cyqsimon commented 1 year ago

I can just update this PR once the other one is merged, do u agree?

Sure, although in that case you will have to modify a few lines in builder.py and install.sh. Shouldn't be too hard though.

Also I just started coughing really bad today, likely COVID. So you probably have to wait a few days, sorry.

ciriousjoker commented 1 year ago

Dw i'll just fast forward my branch. I've waited 2 years for this, a couple more days won't be an issue ;)

I wish you all the best and get well soon!

cyqsimon commented 1 year ago

@ciriousjoker reorganisation's done. Icon is now at /files/res/nibbler.png. Feel free to ask me if there's something you aren't sure about the scripts.

ciriousjoker commented 1 year ago

@cyqsimon

I pushed the icon files and added it to builder.py. Can you please check if install.sh needs to be changed as well? I didn't know what to do there.

Also, since you mentioned credits after my original comments, I've also included this line in the readme:

Icon design by ciriousjoker based on this.

For windows users that line won't make much sense though. Do you know how to use that icon for Windows as well?

cyqsimon commented 1 year ago

Can you please check if install.sh needs to be changed as well?

Yes. I've submitted a PR onto your branch: https://github.com/ciriousjoker/nibbler/pull/1.

Do you know how to use that icon for Windows as well?

On Windows the icon is directly built into the exe as a resource. See here. I'm not confidently proficient in Python, so I'll leave this to @rooklift.

ciriousjoker commented 1 year ago

Now this should be ready to merge.

rooklift commented 1 year ago

Presumably I'll also have to remake nibbler-2.3.9-linux.zip so as to include the SVG file?

cyqsimon commented 1 year ago

Presumably I'll also have to remake nibbler-2.3.9-linux.zip so as to include the SVG file?

That would be kind of messy, modifying a release retroactively. I think it would be best for the SVG files to simply be included in the next release.

Of course if you want to make a new release just for this that would be okay too.

rooklift commented 1 year ago

Is the install script happy enough for the svg file not to exist?

cyqsimon commented 1 year ago

Is the install script happy enough for the svg file not to exist?

Nope, it will error, so it would be a good idea to rebuild the release. But retroactively editing the release is kind of messy, because there will be differences between the source archive at tag and the release .zip, and then the changed file hash may cause confusion downstream, etc.

It's probably the easiest to just make a new patch release, 2.3.10 or something.