[styles] attraction=animal as a standalone tag #7716

map-per · 2024-03-28T15:07:31Z

map-per commented

2024-03-28 15:07:31 +00:00

attraction=* (https://wiki.openstreetmap.org/wiki/Key:attraction) is not a sub key of tourism=attraction (https://wiki.openstreetmap.org/wiki/Tag:tourism%3Dattraction) .
Therefore the style was modified to render attraction=animal without tourism=attraction.

before	now

before	now

(also removed the priority values for tourism-attraction-specified, as there is no style specified)

closes #5911 attraction=* (https://wiki.openstreetmap.org/wiki/Key:attraction) is not a sub key of tourism=attraction (https://wiki.openstreetmap.org/wiki/Tag:tourism%3Dattraction) . Therefore the style was modified to render attraction=animal without tourism=attraction. | before | now | |--------|--------| | ![Bildschirmfoto vom 2024-03-28 12-45-18](https://github.com/organicmaps/organicmaps/assets/79519062/3126fe7b-906a-415c-b294-fcaa964c74b0) | ![Bildschirmfoto vom 2024-03-28 19-31-21](https://github.com/organicmaps/organicmaps/assets/79519062/e852665b-49a5-4862-9ed1-ff6319299af7) | | before | now | |--------|--------| | ![Bildschirmfoto vom 2024-03-28 19-47-06](https://github.com/organicmaps/organicmaps/assets/79519062/04d1a5b7-93e0-42ef-a22d-7672aa6f56b8) | ![Bildschirmfoto vom 2024-03-28 19-32-49](https://github.com/organicmaps/organicmaps/assets/79519062/f1f8636e-b56e-493e-85f0-c953b61a61bd) | (also removed the priority values for tourism-attraction-specified, as there is no style specified)

👍 3

biodranik (Migrated from github.com) approved these changes 2024-03-28 22:47:38 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! Maybe some day we can make separate icons for each animal )

@pastk PTAL

Thanks! Maybe some day we can make separate icons for each animal ) @pastk PTAL

vng (Migrated from github.com) approved these changes 2024-03-29 02:07:42 +00:00

pastk requested changes 2024-03-29 09:05:04 +00:00

pastk left a comment

Please amend types_strings.txt accordingly and regenerate strings.

Also you may want to add attraction-animal into editor.config :)

Please amend `types_strings.txt` accordingly and regenerate strings. Also you may want to add `attraction-animal` into `editor.config` :)

data/mapcss-mapping.csv

pastk commented

2024-03-29 08:55:41 +00:00

please remove specified completely, there are no such values

please remove `specified` completely, there are no such values

data/styles/clear/include/Icons.mapcss

					
				@ -691,9 +692,8 @@ node|z16-[tourism=theme_park],

				area|z16-[tourism=theme_park]

pastk commented

2024-03-29 09:00:01 +00:00

nit: trailing comma for consistency

also note that its better to use just "node" line both here and above around line 460
duplicating "node" & "area" lines is redundant in 95% cases

nit: trailing comma for consistency also note that its better to use just "node" line both here and above around line 460 duplicating "node" & "area" lines is redundant in 95% cases

biodranik (Migrated from github.com) reviewed 2024-03-29 09:23:09 +00:00

data/mapcss-mapping.csv

biodranik (Migrated from github.com) commented

2024-03-29 09:23:09 +00:00

@pastk is it hard to write some kind of validator/check script for issues like this one?

pastk reviewed 2024-03-29 10:01:08 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-29 10:01:08 +00:00

actually we have a lot of types added into the mapping ("for the future") but never used...

pastk reviewed 2024-03-29 10:13:49 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-29 10:13:48 +00:00

on the other hand there are various "service" and "non-drawable" types there like hwtag-oneway, cuisine-, wheelchair-, etc...

so the information of the actual use (or absence of any use) is ATM spread in different files and sometimes is hardcoded

I think that type traits like service / drawable / searchable / addable? / etc. should be clearly and explicitly configured for each type in the mapping config (we'll extend the syntax, maybe yaml)

@vng WDYT?

on the other hand there are various "service" and "non-drawable" types there like hwtag-oneway, cuisine-*, wheelchair-*, etc... so the information of the actual use (or absence of any use) is ATM spread in different files and sometimes is hardcoded I think that type traits like service / drawable / searchable / addable? / etc. should be clearly and explicitly configured for each type in the mapping config (we'll extend the syntax, maybe yaml) @vng WDYT?

pastk reviewed 2024-03-29 11:30:25 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-29 11:30:25 +00:00

organicmaps/organicmaps#7728

- https://git.omaps.dev/organicmaps/organicmaps/issues/7728

pastk reviewed 2024-03-30 10:15:19 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-30 10:12:35 +00:00

deprecated:tourism|attraction|specified:03.2024;[tourism=attraction][attraction=specified];x;name;int_name;1237;x

to keep date and context

```suggestion deprecated:tourism|attraction|specified:03.2024;[tourism=attraction][attraction=specified];x;name;int_name;1237;x ``` to keep date and context

data/strings/types_strings.txt

pastk commented

2024-03-30 10:13:22 +00:00

please delete also

pastk reviewed 2024-03-30 19:36:23 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-30 19:36:23 +00:00

deprecated:tourism|attraction|specified:03.2024;[tourism=attraction][attraction=specified];x;name;int_name;1237;tourism|attraction

sorry tests are failing because of my mistake - there should be no trailing "x"

actually to be super safe I now suggest replacing the old type with the generic tourism|attraction, just in case there were some rare cases in some maps

```suggestion deprecated:tourism|attraction|specified:03.2024;[tourism=attraction][attraction=specified];x;name;int_name;1237;tourism|attraction ``` sorry tests are failing because of my mistake - there should be no trailing "x" actually to be super safe I now suggest replacing the old type with the generic `tourism|attraction`, just in case there were some rare cases in some maps

pastk reviewed 2024-03-31 14:13:42 +00:00

data/mapcss-mapping.csv

pastk commented

2024-03-31 14:13:42 +00:00

@map-per please regenerate_drules also

@map-per please re`generate_drules` also

pastk approved these changes 2024-03-31 19:46:13 +00:00

pastk left a comment

LGTM
Many thanks!

LGTM Many thanks!

fitojb (Migrated from github.com) reviewed 2024-03-31 20:58:27 +00:00

android/app/src/main/res/values-es-rMX/strings.xml

fitojb (Migrated from github.com) commented

2024-03-31 20:58:27 +00:00

Please remove es-MX; no different from es

fitojb (Migrated from github.com) reviewed 2024-03-31 21:03:10 +00:00

android/app/src/main/res/values-ca/strings.xml

fitojb (Migrated from github.com) commented

2024-03-31 21:03:10 +00:00

Recinte d'animals

fitojb (Migrated from github.com) reviewed 2024-03-31 21:03:53 +00:00

android/app/src/main/res/values-pt-rBR/strings.xml

fitojb (Migrated from github.com) commented

2024-03-31 21:03:52 +00:00

This line can be removed too, general pt will suffice

This line can be removed too, general `pt` will suffice

map-per reviewed 2024-04-01 06:28:10 +00:00

android/app/src/main/res/values-es-rMX/strings.xml

map-per commented

2024-04-01 06:28:10 +00:00

@pastk: How is this usually done in OM?

pastk reviewed 2024-04-01 08:22:59 +00:00

android/app/src/main/res/values-es-rMX/strings.xml

pastk commented

2024-04-01 08:22:59 +00:00

just remove this like in types_strings.txt organicmaps/organicmaps#7716/commits/ff73ddd3bc49fb5d84bcfe0b093ac134bfed200b#diff-dc83cee833f96d4ab35de9bc5adb183c39800fe95659d682096c694696dc9aea

@fitojb its better to leave review comments against this file as its the source (android and iphone localizations files are generated automatically from it)

just remove this like in `types_strings.txt` https://git.omaps.dev/organicmaps/organicmaps/pulls/7716/commits/ff73ddd3bc49fb5d84bcfe0b093ac134bfed200b#diff-dc83cee833f96d4ab35de9bc5adb183c39800fe95659d682096c694696dc9aea @fitojb its better to leave review comments against this file as its the source (android and iphone localizations files are generated automatically from it)

vgrigoriu (Migrated from github.com) reviewed 2024-04-01 08:53:03 +00:00

data/categories.txt

vgrigoriu (Migrated from github.com) commented

2024-04-01 08:53:03 +00:00

ro:Incintă pentru animale

```suggestion ro:Incintă pentru animale ```

jimcarst (Migrated from github.com) approved these changes 2024-04-01 13:32:48 +00:00

jimcarst (Migrated from github.com) left a comment

nl OK

pastk requested changes 2024-04-04 10:13:39 +00:00

data/strings/types_strings.txt

pastk commented

2024-04-04 10:05:26 +00:00

Better remove it as duplicate.

pastk commented

2024-04-04 10:06:42 +00:00

pt-BR is supported, pt-rBR is not (its an android-specific notation).

Anyways its a duplicate so remove it please

`pt-BR` is supported, `pt-rBR` is not (its an android-specific notation). Anyways its a duplicate so remove it please

iphone/Maps/LocalizedStrings/es-MX.lproj/Localizable.strings

pastk commented

2024-04-04 10:11:30 +00:00

somehow unrelated strings are overwritten;
its likely as issue with your local twine version;
please ensure your twine submodule is up-to-date (should point to a9a97d1 Use custom om suffix for twine version), remove the locally installed version (check via gem list) and remove the tools/twine/twine-*.gem file and then re-run generate_localization.sh - it'll install the right version

@RedAuburn had the same issue before

somehow unrelated strings are overwritten; its likely as issue with your local twine version; please ensure your twine submodule is up-to-date (should point to `a9a97d1 Use custom om suffix for twine version`), remove the locally installed version (check via `gem list`) and remove the `tools/twine/twine-*.gem` file and then re-run `generate_localization.sh` - it'll install the right version @RedAuburn had the same issue before

matheusgomesms (Migrated from github.com) reviewed 2024-04-04 15:24:46 +00:00

data/categories.txt

matheusgomesms (Migrated from github.com) commented

2024-04-04 15:24:46 +00:00

```suggestion ```

matheusgomesms (Migrated from github.com) reviewed 2024-04-04 15:25:10 +00:00

matheusgomesms (Migrated from github.com) left a comment

This r-BR is wrong, and is duplicate of pt, so no need to add it.

matheusgomesms (Migrated from github.com) approved these changes 2024-04-04 15:42:04 +00:00

pastk reviewed 2024-04-04 15:44:06 +00:00

data/strings/types_strings.txt

pastk commented

2024-04-04 15:43:41 +00:00

ah, please remove this ref, I haven't noticed it before :/

it basically means "take translation from tourism.attraction if they're not specified here explicitly"

that should fix wrong translations for pt-BR and es-MX

ah, please remove this `ref`, I haven't noticed it before :/ it basically means "take translation from tourism.attraction if they're not specified here explicitly" that should fix wrong translations for pt-BR and es-MX