Feature #425: add extends quest crossing marking

[ravenfeld]

fixes #425

Possibility of activating an extension for road markings

• [x] Adding a preference
• [x] New screen with images
• [x] Redefinition of a form for Yes No

[mcliquid]

Nice, thanks! I have just tested the Quest and noticed some things:

• The procedure with the setting to overwrite the quest is unusual, as this has never been done before. I like it, but it's difficult to find at first. What do others think?
• I would think about writing the individual values on the pictures, otherwise I'll struggle to find the right one. For example, the distinction between zebra and ladder is difficult if you don't know.
• Most importantly, if the "Expert Quest" is activated, I think it should be possible to overwrite already mapped crossing:markings=yes.

[ravenfeld]

Nice, thanks! I have just tested the Quest and noticed some things:

• The procedure with the setting to overwrite the quest is unusual, as this has never been done before. I like it, but it's difficult to find at first. What do others think?
• I would think about writing the individual values on the pictures, otherwise I'll struggle to find the right one. For example, the distinction between zebra and ladder is difficult if you don't know.
• Most importantly, if the "Expert Quest" is activated, I think it should be possible to overwrite already mapped crossing:markings=yes.

I think the person will have to rescan for it to be taken into account. (crossing:markings=yes)

Yes, I don't know where to put the parameter.

[mcliquid]

I've already restarted the app incl. rescan. There are two crossings at this location: Quest activated: https://github.com/Helium314/SCEE/assets/3351668/76f59792-2881-4057-a9ec-582f24090814 The right crossing with crossing:markings=yes isn't showed as a quest: https://github.com/Helium314/SCEE/assets/3351668/f70fa7b2-0422-4cfd-9b45-95dabb612372 The left one is: https://github.com/Helium314/SCEE/assets/3351668/e4101dc2-2e37-4977-9b50-4379e58cf199

[ravenfeld]

I've already restarted the app incl. rescan. There are two crossings at this location: Quest activated: https://github.com/Helium314/SCEE/assets/3351668/76f59792-2881-4057-a9ec-582f24090814 The right crossing with crossing:markings=yes isn't showed as a quest: https://github.com/Helium314/SCEE/assets/3351668/f70fa7b2-0422-4cfd-9b45-95dabb612372 The left one is: https://github.com/Helium314/SCEE/assets/3351668/e4101dc2-2e37-4977-9b50-4379e58cf199

Yes, it's normal, it's not yet coded to change the filter depending on the function, and I don't know if it's feasible.

[mcliquid]

@Helium314 Perhaps you can provide some support here? Otherwise I would simply separate the quests, even if it means more effort for the user. Then the first quest would be to say "Yes, there are markerings" (crossing:markings=yes) and the second quest would be "Here is a zebra marking" (crossing:markings=zebra).

[ravenfeld]

Thank you for your feedback @mcliquid

• [x] I modified the rendering, changing a line from 4 to 3 to add the labels.
• [x] I've also understood how to set a preference for a specific quest and especially for those that don't have one. Now this recipe has a parameter icon and you are asked to activate or deactivate the mode.

[mcliquid]

Current screenshot with text: https://github.com/Helium314/SCEE/assets/3351668/61f4a11f-0890-4194-84d4-1c0a9a5db4f1

Very nice, the Quest works very well for me. The new setting is definitely more comfortable than the previous implementation. As an extra addition, you could add a check_date:crossing with the current date when adding the information, but that would only be a bonus. (https://taginfo.openstreetmap.org/keys/check_date:crossing#values) This is implemented in the Surface Quest for example.

[ravenfeld]

Current screenshot with text: https://github.com/Helium314/SCEE/assets/3351668/61f4a11f-0890-4194-84d4-1c0a9a5db4f1

Very nice, the Quest works very well for me. The new setting is definitely more comfortable than the previous implementation. As an extra addition, you could add a check_date:crossing with the current date when adding the information, but that would only be a bonus. (https://taginfo.openstreetmap.org/keys/check_date:crossing#values) This is implemented in the Surface Quest for example.

Don't worry, it's added.

[mcliquid]

@Helium314 I would give the code review in your hands 👋

[mcliquid]

@ravenfeld Instead of a general check_date it should be check_date:crossing.

[mcliquid]

Anything missing here?

[ravenfeld]

I would say the licence for the images and can you try to reduce them?

[mcliquid]

I've started my "regular" action to create the Android images (now JPG instead of PNG). I get 318 KB in total. Does this improve anything? crossing_markings.zip

[ravenfeld]

Thank you very much, it doesn't change anything this time so that means it was already good.

[Helium314]

No comments to add to older PRs, so I'll continue with this one:

• Why use this unusual style of completely changing the quest via quest settings, instead of adding a new quest?
  • This has the side effect of a potentially wrong question, and more concerning, a wrong changeset comment.
• Using crossingFilter get() = ... creates the ElementFilterExpression on every single check (quest scan and element edit). Unless you have a really really good reason to do it, this is not acceptable.
• Is anything changed with excludedWaysFilter? On first glance the diff looks like it's a formatting change, which should a) not be in a PR changing code, and b) not happen in SCEE because it unnecessarily creates differnces to SC code.
• The images should be vector graphics if possible
• Always adding a check_date is not in line with other quests, and was seen as not desired by community (you can find more about this somewhere in SC issue tracker)
• The quest seems to allow only a single answer, but combinations other than zebra;dots can also exist.
• Is there a particular reason to exclude lines:paired?

Otherwise it looks fine (I don't think names are necessary, but can't hurt).

[mcliquid]

• The images should be vector graphics if possible

The source images from the wiki are also pixel graphics. I could redraw all the graphics as vectors by hand, but that would be a lot of work. If it's really worth it, I'll do it.

• The quest seems to allow only a single answer, but combinations other than zebra;dots can also exist.

I agree. Wouldn't be a necessary feature for me, as you can also edit it manually in SCEE. But if it doesn't take much effort, it makes sense.

• Is there a particular reason to exclude lines:paired

I had provided the list of possible choices and had orientated myself according to the most common 10 ones in taginfo. But I am of course open to more options.

[Helium314]

The source images from the wiki are also pixel graphics

Maybe the author also has vector graphics, but didn't upload them?

But if it doesn't take much effort, it makes sense.

I think you just need to change the number of allowed answers in the quest form, and in some place join the answers with ;

according to the most common 10 ones in taginfo.

Oh, if it has clearly fewer uses then I guess it's fine to omit it.

[mcliquid]

Maybe the author also has vector graphics, but didn't upload them?

I've just asked the author, hopefully we get them :)

[ravenfeld]

No comments to add to older PRs, so I'll continue with this one:

• Why use this unusual style of completely changing the quest via quest settings, instead of adding a new quest?

  • This has the side effect of a potentially wrong question, and more concerning, a wrong changeset comment.
• Using crossingFilter get() = ... creates the ElementFilterExpression on every single check (quest scan and element edit). Unless you have a really really good reason to do it, this is not acceptable.
• Is anything changed with excludedWaysFilter? On first glance the diff looks like it's a formatting change, which should a) not be in a PR changing code, and b) not happen in SCEE because it unnecessarily creates differnces to SC code.
• The images should be vector graphics if possible
• Always adding a check_date is not in line with other quests, and was seen as not desired by community (you can find more about this somewhere in SC issue tracker)
• The quest seems to allow only a single answer, but combinations other than zebra;dots can also exist.
• Is there a particular reason to exclude lines:paired?

Otherwise it looks fine (I don't think names are necessary, but can't hurt).

I'll look at it after my holidays but I haven't done another quest because that's what you recommended in the issue #425 .

[Helium314]

You're right, thanks. Though at least the changeset comment should still reflect the new tagging possibilities. Possibly you can make it a little more gerneric.

[StenSoft]

Here are the vector drawables: https://github.com/ravenfeld/SCEE/pull/3

[ravenfeld]

Everything has now been converted to svg

[Helium314]

Thanks, only thing missing now is the more generic changeset comment as in https://github.com/Helium314/SCEE/pull/572#issuecomment-2232319315 ("Specify whether pedestrian crossings have markings" is not quite correct any more)

[ravenfeld]

Could you rephrase that? I didn't understand what I should put in generic.