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 map_type and map_size quests #482

Merged
merged 13 commits into from
Nov 17, 2023
Merged

Add map_type and map_size quests #482

merged 13 commits into from
Nov 17, 2023

Conversation

wielandb
Copy link

@wielandb wielandb commented Nov 2, 2023

This PR adds two quests:

  • What kind of map is this?
  • What area is covered by this map?

The values are taken from the wiki pages.

Would like thoughts/comments on the following things:

  • Do we need custom icons for the quests? Or is the board_type icon okay?
  • Are the descriptions of the answers okay? Or do we need pictures for understandability?

photo_2023-11-02_21-10-51
photo_2023-11-02_21-10-53

@mcliquid
Copy link

mcliquid commented Nov 2, 2023

Nice quests!

  • Do we need custom icons for the quests? Or is the board_type icon okay?

This would make the Quest much more recognizable?

  • Are the descriptions of the answers okay? Or do we need pictures for understandability?

For me, pictures would be much quicker to recognize for a decision.

@wielandb
Copy link
Author

wielandb commented Nov 3, 2023

Added quest icons.

@wielandb
Copy link
Author

wielandb commented Nov 3, 2023

The wiki doesn't really provide images we could use. I would need to find some examples myself if the images are deemed neccecary for merge of this PR.

@wielandb
Copy link
Author

wielandb commented Nov 3, 2023

Just as a heads-up, after adding the icons, the app builds succesfully but crashes on startup. I have no idea how I did this since I only added the icons, but I would not consider this PR "tested" anymore.

@Helium314
Copy link
Owner

Or do we need pictures for understandability?

I think for map_size descriptions are enough, but images would be helpful for map_type.

crashes on startup

It doesn't look like there is an error in your last commit. What does the log say?

@wielandb
Copy link
Author

wielandb commented Nov 3, 2023

I think for map_size descriptions are enough, but images would be helpful for map_type.

Okay, I'll be on the hunt for some example pictures.

It doesn't look like there is an error in your last commit. What does the log say?

I found the problem. When pulling the latest version from upstream, a new quest with the same ordinal I used was pulled in. Fixed that.

@wielandb wielandb marked this pull request as draft November 3, 2023 19:33
@wielandb
Copy link
Author

wielandb commented Nov 9, 2023

Added images.
photo_2023-11-09_11-07-15

@wielandb
Copy link
Author

wielandb commented Nov 9, 2023

One image was taken from the OSM Wiki and is licensed under CC-BY. Do we have a file where we name the authors in SCEE?

@wielandb wielandb marked this pull request as ready for review November 9, 2023 10:11
@mcliquid
Copy link

mcliquid commented Nov 9, 2023

Do we have a file where we name the authors in SCEE?

Here you go: https://github.com/Helium314/SCEE/blob/modified/app/src/main/res/authors.txt

I would suggest this one for the toposcope image: https://commons.wikimedia.org/wiki/File:Melshornet03112006GunnarHeltne.jpg

Maybe also to replace with the one in the wiki.

I've added some missing ones here: https://wiki.openstreetmap.org/wiki/Tag:information%3Dmap

@wielandb
Copy link
Author

wielandb commented Nov 9, 2023

I've added some missing ones here: https://wiki.openstreetmap.org/wiki/Tag:information%3Dmap

Oh theese are way better than mine! I think I'll use them.

@wielandb
Copy link
Author

wielandb commented Nov 9, 2023

Images replaced and licenses added.
Should be ready for review now.

@Helium314
Copy link
Owner

I did not yet test it, but some comments:

  • could you convert the png images to jpg if there is a reasonable decrease in size?
  • raster images should use density qualifiers, i.e. use hdpi and similar folders for reduced size images, you can compare with images from other quests
  • map_type_1.png is missing in authors.txt
  • MapType appears to use placeholders, e.g. ZERO("0") instead of e.g. TOPOGRAPHIC("tope")

@wielandb
Copy link
Author

  • could you convert the png images to jpg if there is a reasonable decrease in size?
  • raster images should use density qualifiers, i.e. use hdpi and similar folders for reduced size images, you can compare with images from other quests
  • map_type_1.png is missing in authors.txt
  • MapType appears to use placeholders, e.g. ZERO("0") instead of e.g. TOPOGRAPHIC("tope")

Everything done.

@Helium314 Helium314 merged commit fd9c209 into Helium314:modified Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants