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

Improve "Select Frame" #5303

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Improve "Select Frame" #5303

merged 2 commits into from
Aug 13, 2024

Conversation

mrbuds
Copy link
Contributor

@mrbuds mrbuds commented Aug 9, 2024

This allows for example to select Party1 default frame which has no name

image

image

_G["PartyFrame.MemberFrame1"] doesn't exist but _G["PartyFrame"].MemberFrame1 does

@mrbuds
Copy link
Contributor Author

mrbuds commented Aug 9, 2024

I didn't apply this to exec_env_builtin, i'm unsure if it should be done

@mrbuds
Copy link
Contributor Author

mrbuds commented Aug 9, 2024

example aura

!WA:2!1r1tpUrnu8eLdGciieqlW29qOiQufsR0wbPWPsMWmSfLTjDYSSLtt8m2tgdo2w2EYUPhJ4aN3lCHt5miHYboWX(jyueFcYhH9tap7jWwHAUmZZwV)(Z)(9Qh0EwBCB8pFFLOWWOCIA45rdE8t8vxnmlttm1xJPAjdTiICL5fr(JJ68jNCCbNA2OZryXLFFLFhvBnINMluJeuUjPV)tI8dLDEPGJZeQzitSm2qNr2DQUSvAH2iMz94CjgziHK5eUzlQaUfzOPxqXMCVEWrfk1qfCD3qTbPmZVzktCzCMcbPZSqs2ee27m)X(d87hnmuHfXwhMS25wvSE6CXLRUno5NncY0IaR9XNrMLquo7to(ucIb1fP6l4gKfzU9QTfAIl3XPcMqn5ANTRfskmgbF4CIcg7v36Z3E6J(1Jo6RQvREJtF0FD4HGvJgjzuovNFW2CkMeJymxs1t8GRnnxAu0Ptjk9BDp1oZ)0Zwepl8lBBBcuIwOsSn2zcmzYwDrId(gxKLrVADC)EJJIhh1lmA7TU7CT8nKV35qE60phzbwIIQb0w)FPyKIaPiC8i)bdwQLeg7XyDZL20yR)KYpkCkWAKHCaV0nxHjjqrJG(tDQ)GrbNp46c(U(UP8DS4)Csu1zBh8B1K35vspQmcTSeS8E7NbXitrPlIZycHk)AeNAPlcE3Y3o4feKMm2Oi8PM8xRSJhxWjBWfkNhUxkLMKk4y9sRR2UUSR3m4DoO8ZHak7w(WYVO8lH)35)FJxgWiKTcu0N35PfimKuuNOOvaWfsMAjOhEqzNnWXeu6pAXio2Jjq4GedIbi7bHZkygAtpavtpOSxZWugsRTwEA6ZjwJL2Imgo8H70zpRsN1yLrKcSlnuNpO(V)hYwvYohR19r((7HsVs56o7S6za4uE39JTZe4yyUitUbuZefhX(UQA(lYpE)rjvKuQ1RgRSyl1PwHPZjaaEVSvRo1Ssa5NU)KGxaekAASjxra1kdVq9dWkcA2cVb(brVYDkvBK2DQS7DPyp7QQLxku4lui5Yl2zS2P6bMb8M9U38sqNfvwxT6WTfjbgGm60MpOGIxNK80e)RKFZ9F4jBG9sqmdLvZMAX)UMS6vQVBuRxVEJvG4HGDlV(P3CnLx1BqqnVwtyzU1KLVEB9d(AY82Z)7N9p)

image

@emptyrivers
Copy link
Contributor

since recursive frame identifiers are "just strings" this change has a small chance of making booby traps:

  • user anchors aura to "Foo.Bar" recursive frame name
  • addons update, someone creates a frame with name "Foo.Bar"
  • weakauras notices "Foo.Bar" and anchors aura to it
  • user correctly notes that weakauras didn't anchor aura to what they intended, opens ticket

i don't know how likely this is to ever actually happen, but i think we could head it off by teaching weakauras how to recognize recursive frame names in a better way (e.g. if frame selector enters recursive branch, then the anchor type is "SELECTRECURSIVEFRAME" or whatever instead of "SELECTFRAME")

@mrbuds
Copy link
Contributor Author

mrbuds commented Aug 9, 2024

since recursive frame identifiers are "just strings" this change has a small chance of making booby traps:

* user anchors aura to "Foo.Bar" recursive frame name

* addons update, someone creates a frame with name "Foo.Bar"

* weakauras notices "Foo.Bar" and anchors aura to it

* user correctly notes that weakauras didn't anchor aura to what they intended, opens ticket

i don't know how likely this is to ever actually happen, but i think we could head it off by teaching weakauras how to recognize recursive frame names in a better way (e.g. if frame selector enters recursive branch, then the anchor type is "SELECTRECURSIVEFRAME" or whatever instead of "SELECTFRAME")

What do you think about making recurseGetName insert an invisible characters (\0) before names found by their parent's key?

@mrbuds
Copy link
Contributor Author

mrbuds commented Aug 9, 2024

mhmm not \0 as it end the string, but you get the idea

@InfusOnWoW
Copy link
Contributor

InfusOnWoW commented Aug 9, 2024

I personally think "." is fine, the clashes are imho pretty theoretical. And thus the breakage if we change the rules are also pretty remote. If we want to do it "perfectly" we'd introduce a escape character, e.g. \, add a Modernize that looks at the existing anchors, escapes any "." or "" found and then that's perfectly backward compatible.

But that sounds like overkill.

@mrbuds
Copy link
Contributor Author

mrbuds commented Aug 10, 2024

But that sounds like overkill.

I agree with that

@InfusOnWoW
Copy link
Contributor

@emptyrivers are you okay with merging this? Or do you think we should be more careful in doing this?

@emptyrivers
Copy link
Contributor

if i truly cared about the edge case, i would've used the review type that blocks merging 😄

@InfusOnWoW InfusOnWoW merged commit 3a9c8e4 into WeakAuras:main Aug 13, 2024
3 checks passed
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