Migration of palette elements #17633
Conversation
mike-spa
force-pushed
the
migrationOfPaletteElements
branch
from
a0e9295
to
0a665ff
Compare
src/palette/internal/palettecell.cpp
Outdated
| @@ -224,6 +230,41 @@ bool PaletteCell::read(XmlReader& e) | |||
| return add && element; | |||
| } | |||
|
|
|||
| void PaletteCell::migrateOldItemIfNeeded() | |||
There was a problem hiding this comment.
I think it's better to add a separate class with name like "PaletteCompat" and add this function to it as static with input arg and output (new item);
In the future, we may add other functions to this class and we will see what kind of compatibility we support.
There was a problem hiding this comment.
Great, thanks! Will do
There was a problem hiding this comment.
@igorkorsukov done! 👍
mike-spa
force-pushed
the
migrationOfPaletteElements
branch
3 times, most recently
from
3927fbf
to
e1e2236
Compare
mike-spa
force-pushed
the
migrationOfPaletteElements
branch
3 times, most recently
from
9f696ba
to
9defca8
Compare
|
@igorkorsukov the builds and tests are now all passing, feel free to merge this, if it's ok |
I think it needs to be tested. @DmitryArefiev FYI |
|
@mike-spa So the way to check is run 4.0.2 build reset text palette (should be 'Expression'), then run PR's build - it should show 'expression' without Text palette reset, correct? |
|
I found how to check it. Run 4.0.2 build (dev), reset ornament palette, run PR's build, add an ornament to score, check available options in Properties: they should have an updated style (without Reset ornament palette). Tested on Mac13 - PASS @mike-spa I see the branch has a conflicts, should be resolved before merge? |
@DmitryArefiev yes, sorry I missed this comment. That's exactly the way to test it, and also the other one you did with expressions. Thank you for testing this. As for the conflict, I'll rebase this and fix it asap |
Introduce PaletteCompat class
mike-spa
force-pushed
the
migrationOfPaletteElements
branch
from
9defca8
to
ed69724
Compare
|
done! |
|
Cool! Thanks! Tested #17581 on Win10, Mac13 - PASS Merged |
Resolves: #17581
@cbjeukendrup @igorkorsukov I've attempted a very basic solution here which seems to work nicely. Let me know if it's ok or we need something different.