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

Add .desktop file and placeholder icon. #81

Merged
merged 1 commit into from Nov 10, 2014

Conversation

boombatower
Copy link
Contributor

When packaging openage it would be nice to have a common .desktop and icon file to use.

@TheJJ
Copy link
Member

TheJJ commented Nov 2, 2014

Oh gaben, bringer of the half life 3, thou thy lordship is perfect for our project, there must not be binary files in the repo where they can be avoided. The clean solution would be to add a install rule that uses imagemagick or similar to cut the already existing gaben image to icon format.

The real logo should be some epic svg so we'll never have to provide higher resolution icons including the day we got 9001k screens.

@boombatower
Copy link
Contributor Author

I'll look into adding a rule. Also, a preview:

@boombatower
Copy link
Contributor Author

I recommend that I rebase the branch before it is pulled so there is no copy of binary image in history. Also not sure how close I am with cmake stuff or how to get it included since it does not seem to be executed. I'll leave it here and wait for input.

@mic-e
Copy link
Member

mic-e commented Nov 2, 2014

Looking forward to the nightlies. Also I love gaben.

However, the dist folder shouldn't be inside assets. Instead, create a folder dist/, with a subfolder opensuse (obviously gaben isn't distribution-specific and should be directly in dist/.

Clarification: the assets folder is for all files that are used by openage at run-time, and is installed to /usr/share/openage.

Avoiding binaries in the git history is indeed a goal.

@TheJJ
Copy link
Member

TheJJ commented Nov 2, 2014

The .desktop file is for all distros, you can place it in dist/openage.desktop i'd say. Distro specific stuff should be put to dist/$distroname/, but we have to condider whether we want to put the package building script in our repo.

I think putting distro build scripts into the repo is a bad idea and shouldn't be done. The files will outdate and just clobber the repo, instead we could create a new repo where distroscript are stored in.

Don't misunderstand me, packaging related files like the .desktop is suited for all distros and can be put inside the repo.

@zuntrax
Copy link
Contributor

zuntrax commented Nov 2, 2014

You should also add ImageMagick to the dependencies in building.md

@zuntrax zuntrax added the packaging Bundling openage for distribution label Nov 2, 2014
@TheJJ
Copy link
Member

TheJJ commented Nov 2, 2014

Ugh, another dependency... I hope we could avoid that somehow, especially as other platforms might have difficulties with using or having imagemagick available.

@LordAro
Copy link
Contributor

LordAro commented Nov 2, 2014

I see no issue with storing images in the repo, if they're necessary

@phrohdoh
Copy link
Contributor

phrohdoh commented Nov 2, 2014

OS X can get ImageMagick easily through Homebrew, but not all users will want to go that route.

@mic-e
Copy link
Member

mic-e commented Nov 2, 2014

We do have PIL as a depend, right? Maybe some 5-line python script could duplicate the imagemagick functionality?

import PIL.Image
file=PIL.Image.open('/home/mic/git/openage/assets/gaben.png')
file.resize((24, 24), PIL.Image.ANTIALIAS).save('/tmp/tinygaben.png')

Sorry, 3 lines, actually.

@TheJJ
Copy link
Member

TheJJ commented Nov 2, 2014

very good idea, that should be the way to go.

@boombatower
Copy link
Contributor Author

Can someone point me to how I need to hook up cmake stuff (per my 3rd comment). It doesn't seem my CMakeLists.txt is invoked. I imagine I have to hook it up somewhere. I think having dist just contain files needed is fine unless we find a reason to include distro specific stuff.

Moved the rest of this comment to #94.

@franciscod
Copy link
Contributor

@boombatower take a look at this: https://github.com/franciscod/openage/commit/953d7471ebd0e98a93d27dc3ec3fb342ba4d9231

it still depends on imagemagick

@franciscod
Copy link
Contributor

@boombatower
Copy link
Contributor Author

lol excellent commit messages look ma, no imagemagick!

Thanks I'll get it fixed up and assume we should put final version in /dist as discussed.

@franciscod
Copy link
Contributor

hahaha glad you liked it

@boombatower
Copy link
Contributor Author

What I currently see is:

Cropping gaben.png to create placeholder desktop icon...
Traceback (most recent call last):
  File "<string>", line 4, in <module>
  File "/usr/lib64/python3.3/site-packages/PIL/Image.py", line 1558, in save
    fp = builtins.open(fp, "wb")
FileNotFoundError: [Errno 2] No such file or directory: '/home/boombatower/openage/.bin/gcc-release-O2/dist/openage.png'

Creating dist/ in main tree doesn't seem to help. Otherwise all the cmake stuff runs!

@boombatower boombatower force-pushed the desktop-file-plus-icon branch 2 times, most recently from a7112cb to 0fd13dd Compare November 3, 2014 04:55
@franciscod
Copy link
Contributor

what if you do ls /home/boombatower/openage/.bin/gcc-release-O2/ ? my guess is that dist/ doesn't exist :)

@franciscod
Copy link
Contributor

@TheJJ: "The real logo should be some epic svg so we'll never have to provide higher resolution icons including the day we got 9001k screens."

MSG   ERROR: Could not load texture from 'assets/gaben.svg': Unsupported image format
FATAL Exception: Could not load texture from 'assets/gaben.svg': Unsupported image format

NOOOOOOOOOOOOOOOOOOOOOOOOO 😢

gaben.svg

@boombatower
Copy link
Contributor Author

Nothing there, no dist/. That's where I looked before as I printed out the path.

$ ls -l /home/boombatower/openage/.bin/gcc-release-O2/
total 100
drwxr-xr-x  3 boombatower users  4096 Nov  2 22:41 assets
drwxr-xr-x  4 boombatower users  4096 Nov  2 22:47 build
-rw-r--r--  1 boombatower users 30935 Nov  2 22:41 CMakeCache.txt
drwxr-xr-x 35 boombatower users  4096 Nov  2 22:47 CMakeFiles
-rw-r--r--  1 boombatower users  3136 Nov  2 22:32 cmake_install.cmake
-rw-r--r--  1 boombatower users  3871 Nov  2 22:47 codegen_depend_cache
-rw-r--r--  1 boombatower users  1589 Nov  2 22:47 codegen_target_cache
-rw-r--r--  1 boombatower users     0 Nov  2 22:47 codegen_timefile
drwxr-xr-x 12 boombatower users  4096 Nov  2 22:47 cpp
-rw-r--r--  1 boombatower users   324 Nov  2 22:32 CTestTestfile.cmake
-rw-r--r--  1 boombatower users  2236 Oct 26 23:02 DartConfiguration.tcl
-rw-r--r--  1 boombatower users 21222 Nov  2 22:41 Makefile
drwxr-xr-x  4 boombatower users  4096 Nov  2 22:47 py
drwxr-xr-x  3 boombatower users  4096 Oct 26 23:02 Testing

@franciscod
Copy link
Contributor

You can add a line to your CMakeLists.txt:

file(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/dist)

maybe this is redundant... but it'll work

@boombatower
Copy link
Contributor Author

Was wondering if something like that was needed (worked). Ok, I'll clean this up and get things moved around. Also rebase to get rid of image file in history.

@boombatower boombatower force-pushed the desktop-file-plus-icon branch 2 times, most recently from 15051b3 to fcb5c53 Compare November 3, 2014 05:16
@boombatower
Copy link
Contributor Author

Alight, completed all the polishing. Rebased so squash all commits together so the image disappears entirely. :) I'll need to update packaging to use this once merged.

@franciscod Thanks again for assistance.

@boombatower
Copy link
Contributor Author

Also preemptively added quotes per #92.

@franciscod
Copy link
Contributor

you're welcome! (hi welcome, i'm franciscod! (?))

@TheJJ
Copy link
Member

TheJJ commented Nov 3, 2014

LOL @ svg-gaben.

The icon generation looks ok for now. The desktop-file will be non-functional of course as ~/.openage will not be created by make media by default, a manual conversion call will be necessary.

But, i'd say this is good for the first try, 👍.

@boombatower
Copy link
Contributor Author

Per, other issues I have updated .desktop file as follows (rebased to keep ready):

diff --git a/dist/openage.desktop b/dist/openage.desktop
index 5c3c822..e52e180 100644
--- a/dist/openage.desktop
+++ b/dist/openage.desktop
@@ -1,8 +1,10 @@
+#!/usr/bin/env xdg-open
 [Desktop Entry]
 Name=openage
 Comment=Free engine clone of Age of Empires II
-Exec=openage --data="~/.openage/assets" %u
-Terminal=false
+Exec=openage --data=assets %u
+Path=~/.openage/
+Terminal=true
 Type=Application
 Icon=openage
 Categories=Game;StrategyGame;

@TheJJ
Copy link
Member

TheJJ commented Nov 4, 2014

when i take another look at it, it seems to be more suitable if the CMakeLists.txt for the icon conversion would be placed to dist/, do you agree?

@franciscod
Copy link
Contributor

wow, does github autoformat diffs in that way on markdown triple backtick blocks?

@phrohdoh
Copy link
Contributor

phrohdoh commented Nov 5, 2014

@franciscod yes if you use ```diff

@franciscod
Copy link
Contributor

thanks @phrohdoh!

@boombatower
Copy link
Contributor Author

Placed CMakeLists.txt in dist.

@mic-e
Copy link
Member

mic-e commented Nov 10, 2014

👍 (once that grammar issue is fixed)

@boombatower
Copy link
Contributor Author

should be good to go

@boombatower
Copy link
Contributor Author

Just look at gaben looking down over the glory that is openage.

mic-e added a commit that referenced this pull request Nov 10, 2014
Add .desktop file and placeholder icon.
@mic-e mic-e merged commit 78f9599 into SFTtech:master Nov 10, 2014
@boombatower boombatower deleted the desktop-file-plus-icon branch November 10, 2014 22:22
mic-e added a commit that referenced this pull request Nov 12, 2014
@charego
Copy link
Contributor

charego commented Nov 13, 2014

Thanks for the fix.

@boombatower
Copy link
Contributor Author

👍

schets added a commit to schets/openage that referenced this pull request Nov 16, 2014
phew, back to added block allocator

formatting

formatting

redid some of the git stuff, working on the formatting

redid some of the git stuff, working on the formatting

more formatting

more formatting

more formatting

more formatting

more formatting

more formatting

formatting

formatting

formatting

formatting, added default ctor to create

oops

fixed error with block_allocator::create and multiple arguments

added alignment primitives to allocator

allocator used std::allocator, uses hints for where to place memory

removed alignment, wasn't needed. Re-added standard allocator

Fix warnings on 32 bit systems

- Change format specifiers to not make assumptions about integer and
  size_t sizes
- Change audio hash function to shift a different amount based on
  sizeof(size_t)

buildsystem: fixed install path issues from SFTtech#81

added some whitespace

more formatting:

buildsystem: fixed install path issues from SFTtech#81

phew, back to added block allocator

phew, back to added block allocator

formatting

formatting

redid some of the git stuff, working on the formatting

redid some of the git stuff, working on the formatting

more formatting

more formatting

more formatting

more formatting

more formatting

more formatting

formatting

formatting

formatting

formatting, added default ctor to create

oops

allocator used std::allocator, uses hints for where to place memory

removed alignment, wasn't needed. Re-added standard allocator

added some whitespace

formatting

formatting

formatting
schets added a commit to schets/openage that referenced this pull request Nov 16, 2014
phew, back to added block allocator

formatting

formatting

redid some of the git stuff, working on the formatting

redid some of the git stuff, working on the formatting

more formatting

more formatting

more formatting

more formatting

more formatting

more formatting

formatting

formatting

formatting

formatting, added default ctor to create

oops

fixed error with block_allocator::create and multiple arguments

added alignment primitives to allocator

allocator used std::allocator, uses hints for where to place memory

removed alignment, wasn't needed. Re-added standard allocator

Fix warnings on 32 bit systems

- Change format specifiers to not make assumptions about integer and
  size_t sizes
- Change audio hash function to shift a different amount based on
  sizeof(size_t)

buildsystem: fixed install path issues from SFTtech#81

added some whitespace

more formatting:

buildsystem: fixed install path issues from SFTtech#81

phew, back to added block allocator

phew, back to added block allocator

formatting

formatting

redid some of the git stuff, working on the formatting

redid some of the git stuff, working on the formatting

more formatting

more formatting

more formatting

more formatting

more formatting

more formatting

formatting

formatting

formatting

formatting, added default ctor to create

oops

allocator used std::allocator, uses hints for where to place memory

removed alignment, wasn't needed. Re-added standard allocator

added some whitespace

formatting

formatting

formatting

please have actually worked

formatting:

formatting:
inakoll added a commit to inakoll/openage that referenced this pull request Nov 24, 2014
inakoll added a commit to inakoll/openage that referenced this pull request Nov 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Bundling openage for distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants