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 possibility to add/save multiple custom overlays #404

Closed
SLMapper opened this issue Jan 7, 2023 · 25 comments
Closed

Add possibility to add/save multiple custom overlays #404

SLMapper opened this issue Jan 7, 2023 · 25 comments

Comments

@SLMapper
Copy link

SLMapper commented Jan 7, 2023

Use case
Want to use different custom overlay for different use cases and be able to switch between them

Why

  • Defining the overlay can be quite complex, so I want to retain it (this could theoretically be done in any separate text app)
  • Defining two custom overlays to quickly switch between them it currently not possible

Proposed Solution
In the editor:

  • Add field to give it a name
  • Add a delete button

In the list of overlays

  • Store different custom overlays as separate entry
  • Rename the "custom overlay" button to "edit"
  • Make it sensitive to the selected quest
    • will edit the selected overlay
    • only enabled when a custom overlay is selected
@Helium314
Copy link
Owner

I see a few issues here

  • I currently don't want to spend much time on development (sorry, though this will likely improve)
  • The internal structure of SC doesn't allow for simply adding and removing overlays by the user, so there can only be something like "custom overlay 1 / 2 / 3 /..." (though it should be possible to rename them). Anyway, the point is that the number will always be limited.
  • SC will change overlay selection UI at some point, and I really dislike working on UI stuff. Thus I want to avoid doing that work twice, and wait for SC to switch UI first.

Otherwise having multiple custom overlays is definitely something that could be implemented.

@Helium314
Copy link
Owner

Next version will contain a setting to import / export custom overlays (previously they were part of the normal settings export). This should improve switching at least a little bit.

For the multiple overlays, it should actually be possible to add an arbitrary number of them by re-using the same overlay internally and just changing settings. Depending on the to-be-done UI, it might be possible to make it look like having multiple overlays, maybe even with a customizable name.

@SLMapper
Copy link
Author

SLMapper commented Feb 2, 2023

Import export works 🙂 Thank you.
If you / somebody will find the time to make it more convinient like

For the multiple overlays, it should actually be possible to add an arbitrary number of them by re-using the same overlay internally and just changing settings. Depending on the to-be-done UI, it might be possible to make it look like having multiple overlays, maybe even with a customizable name.

it will be great

@Helium314
Copy link
Owner

It's definitely planned, but I'd like to wait for the update to the overlay selection menu, so I don't need to re-do the UI possibly in a few weeks.

@Helium314
Copy link
Owner

This is now implemented in a way that should not need (large) changes when the selection UI changes later. Could you please test it? Link
Everything should work, but if something goes really wrong there is a risk of losing your current custom overlay settings. So it's safer to export your overlays first.

You can add all of your exported overlays by importing them one by one, and selecting "add".

@NotSoImportant
Copy link

It generally works well (with some limitations)! Thank you 🏆

Things I recognized:

  1. importcustom overlay
    • "add" does not work with exported overlays (in old format?) - maybe only once you already have some in the new format
    • "replace" actually replaces content of the first (default) custom overlay, even if it already contained content and even if it had a custom name. this making it look like it did not work when only looking at the list of overlays (and not its content)
  2. custom overlaymanage deleting an entry leads to leaving the context (maybe to not have to clean up when you delete the currently opened one)
  3. overlay selection gets lost in the following case:
    1. have at least 2 custom overlays
    2. have a custom overlay selected (and applied with "ok")
    3. open list of overlays, click on another custom overlay
    4. click "manage"
    5. click "ok" or "cancel"
    6. (open list of overlays again and see that "none" is selected)
  4. when adding a new custom overlay via manageadd sometimes they are copied from existing, sometimes they are empty - have not found out when
  5. when adding a new custom overlay and then pressing "cancel" in the custom overlay dialog, the new custom overlay was still created

@Helium314
Copy link
Owner

"add" does not work with exported overlays (in old format?) - maybe only once you already have some in the new format

This definitely needs to work and should add the imported overlay last. Looks like some bug.

"replace" actually replaces content of the first (default) custom overlay

This is intentional, and I think it's not too surprising that an overlay might be replaced. I also considered removing all existing custom overlays before when choosing replace.

deleting an entry leads to leaving the context (maybe to not have to clean up when you delete the currently opened one)

What did you expect to happen? It could also show the updated manage dialog (and then probably replace cancel with close for the button)

overlay selection gets lost in the following case

Thanks for the steps, I'll look into it. But this might be a case of too much work for fixing a minor inconvenience.

sometimes they are copied from existing

Could it be that you had imported this existing overlay with add before? This could be related to 1.

when adding a new custom overlay and then pressing "cancel" in the custom overlay dialog, the new custom overlay was still created

Since this is only a minor inconvenience, I'd rather only fix this if I find a simple way.

@Helium314 Helium314 reopened this Mar 17, 2023
@Helium314
Copy link
Owner

Helium314 commented Mar 17, 2023

1 . and possibly 4. should be fixed in bb4b83e
5 . and maybe 2. and 3. should be fixed in 39db7a7

I can't test it for the next few days, but if you want to give it another try: https://github.com/Helium314/SCEE/actions/runs/4445442463 / https://github.com/Helium314/SCEE/suites/11626796574/artifacts/603342855

Edit: ignore this version, needs more work.

@NotSoImportant
Copy link

apk seems broken

@Helium314
Copy link
Owner

What seems broken? Can't install?

@NotSoImportant
Copy link

Yes

@Helium314
Copy link
Owner

This is weird, works for me... does the installer show any useful error message indicating the reason?

Anyway, there is another one available: https://github.com/Helium314/SCEE/actions/runs/4456557207 / https://github.com/Helium314/SCEE/suites/11652583573/artifacts/605210224

@NotSoImportant
Copy link

Same again
Screenshot_20230318-211545

@NotSoImportant
Copy link

It only worked for me after reinstalling the app.


Now as expected

  • importreplace will replace all overlays
  • create and cancel will not create a new custom overlay
  • deleting in manage dialog brings up manage dialog again
  • "ok" selects the currently edited custom overlay

Issues

  1. importadd will create a new custom overlay entry, but with copying the content of the first custom overlay instead of the to be imported one
  2. still loosing context / imho not intuitive result after pressing ok, cancel buttons

Suggestion for 2

  • put "manage" button into the list of overlays (or a settings icon button if otherwise too narrow)
  • (rename "custom overlay" button to "customize")
  • clicking any button in these dialogs ("cancel" or "ok" in the custom overlay, "close" in the manage dialog) will bring up the overlay list (like returning to it)
    • By this you will be able to directly see the impact of your chosen action

@Helium314
Copy link
Owner

It only worked for me after reinstalling the app.

This is even weirder. After uninstalling SCEE the package is not corrupt any more?
Actually it looks like mismatching keys, but with a misleading error message, Only... they match, and upgrading works on my phone.
Which Android version are you using? Probably something newer than my Android 9 I guess.

still loosing context / imho not intuitive result after pressing ok, cancel buttons

This is something where too many changes are needed for a relatively small impact. Especially considering that these changes will need to be re-done once the SC UI for overlay selection changes (which was already announced somewhere).
I would accept a PR on this though.

@NotSoImportant
Copy link

Ok, thanks

@Helium314
Copy link
Owner

Helium314 commented Mar 19, 2023

I compared the APK from GitHub workflow https://github.com/Helium314/SCEE/actions/runs/4456557207 with my home-built and couldn't find any difference in signing keys or contained data, so still no idea why it doesn't work for you...

Import should really be fixed now in a7bf7ac

APK should soon be available: https://github.com/Helium314/SCEE/actions/runs/4462128006
But next version will be released soon anyway

@NotSoImportant
Copy link

Importadd works.

But I encountered another bug:

  • when exporting multiple overlays at once and importing them again (with replace), the first custom overlay contains content of the previously selected overlay.

@Helium314
Copy link
Owner

Actually when you select replace, all custom overlay settings are deleted before doing the import. So I have no idea how you could get content of a previous overlay.

Can you share the exact steps to reproduce this?

@NotSoImportant
Copy link

Previous selection is stored in the exported settings

@Helium314
Copy link
Owner

Ah, that's probably an issue with old and new overlays in one file, which was only possible in the testing versions (and files exported with them).
On version upgrade, old overlay settings are converted to new ones to prevent this. But the testing version was still not 52.0.

You can fix such exported overlay files manually by changing custom_overlay_filter to custom_overlayfilter and custom_overlay_color_key to custom_overlaycolor_key, and adding the number to the custom_overlay_indices list.
Just make sure to choose a number that's not already used.

@NotSoImportant
Copy link

NotSoImportant commented Mar 21, 2023

Seems not only limited to testing versions. I can reproduce like this:

  1. fresh install 51.0-beta1
  2. create custom overlay & export it
  3. (repeat 3 times)
  4. uninstall & fresh install 52.0-beta1
  5. import all 3 using add
  6. select one of them
  7. export all together
  8. import with replace
  9. content of the first one is wrong

One exported overlay from 51.0-beta1
overlays boolean settings {} int settings {} long settings {} float settings {} string settings {"custom_overlay_color_key":"","custom_overlay_filter":"nodes, ways, relations with A"} string set settings {}

@NotSoImportant
Copy link

Anyhow. As it is working fine with exports from current version and there was not a huge time in between that should be fine. Thank you

@Helium314
Copy link
Owner

Thanks for the steps and your persistence.
The reason for this is that old (non-indexed) overlay settings where copied to new (indexed) settings, but never removed...

It's fixed now, and in the next version overlays exported with a buggy version will be fixed / import correctly.

@SLMapper
Copy link
Author

🎉 nice job. I love it ❤️

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

No branches or pull requests

3 participants