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

Icon texture does not update on talent or gear swap #749

Closed
Hemario opened this issue Dec 17, 2020 · 5 comments
Closed

Icon texture does not update on talent or gear swap #749

Hemario opened this issue Dec 17, 2020 · 5 comments
Assignees

Comments

@Hemario
Copy link
Contributor

Hemario commented Dec 17, 2020

Describe the bug
Icon texture does not update for spells when they get replaced by another spell due to talent swaps.
Icon texture of trinket does not change when you swap trinket.

To Reproduce
Custom script to test (protection paladin)

Define(blessed_hammer_talent 23469) 
Define(blessed_hammer 204019)
    SpellInfo(blessed_hammer cd=6)
Define(hammer_of_the_righteous 53595)
    SpellInfo(hammer_of_the_righteous cd=6)
    SpellRequire(hammer_of_the_righteous replaced_by set=blessed_hammer enabled=(hastalent(blessed_hammer_talent)))

AddIcon {
    spell(hammer_of_the_righteous)
}
AddIcon {
    Item(Trinket0Slot)
}
AddIcon {
    Item(Trinket1Slot)
}
  1. Change talents from Blessed Hammer to Holy Shield
  2. Change your trinkets

Expected behavior
The icon textures should update according to the newly selected talent or trinket.
After a /reload the item texture is correct.

@johnnylam88
Copy link
Contributor

I've seen this as well when I wear two on-use trinkets and I swap the locations of the two trinkets.

@Hemario
Copy link
Contributor Author

Hemario commented Dec 21, 2020

Even worse for Havoc Demonhunters (after merging Hemario@b2fd91d)

Include(ovale_common)
Include(ovale_demonhunter_spells)

AddIcon {
spell(chaos_strike_havoc)
}

AddIcon {
Spell(blade_dance)
}

Casting meta will not change the texture of the icons

@johnnylam88
Copy link
Contributor

Here is an even simpler example:

AddCheckBox(test_checkbox "test")

AddIcon {
    if CheckBoxOn(test_checkbox) 1
    0
}

Using this script, Ovale will show "0.0" in the icon frame, but if you toggle the checkbox, it doesn't change to "1.0" unless you reload the UI. I've turned on debugging for OvaleCompile and it does say:

OvaleCompile: Ovale_CheckBoxValueChanged: advance age to 4.
OvaleCompile: Script has changed. Evaluating...

@johnnylam88
Copy link
Contributor

I've figured out why this is happening, and I will have a pullup for this issue shortly.

johnnylam88 added a commit to johnnylam88/Ovale that referenced this issue Dec 29, 2020
The "constant" optimization in RecursiveCompute() was returning the
wrong results for "if" nodes when the result was a "value" node
because the cached result from a previous computation wasn't being
updated if the condition's computed result was different than
before.

Remove the "constant" optimization from RecursiveCompute() and add
a new visitor computeUndefined() to visit any nodes of type
"undefined" that were generated by parsing typed functions.

Export type AstUndefinedNode in engine/ast so that in can be used
in engine/runner in computeUndefined().

This fixes part of issue Sidoine#749.
johnnylam88 added a commit to johnnylam88/Ovale that referenced this issue Dec 29, 2020
The idiom that was used to set the texture was unnecessary when
the variables were all local, but is plain wrong when using
"result" fields because it would never overwrite a cached result's
texture. Simplify by using a simple if-else statement to set the
texture.

This fixes the rest of issue Sidoine#749.
johnnylam88 added a commit to johnnylam88/Ovale that referenced this issue Jan 5, 2021
The idiom that was used to set the texture was unnecessary when
the variables were all local, but is plain wrong when using
"result" fields because it would never overwrite a cached result's
texture. Simplify by using a simple if-else statement to set the
texture.

This fixes issue Sidoine#749.
Sidoine pushed a commit that referenced this issue Jan 5, 2021
The idiom that was used to set the texture was unnecessary when
the variables were all local, but is plain wrong when using
"result" fields because it would never overwrite a cached result's
texture. Simplify by using a simple if-else statement to set the
texture.

This fixes issue #749.
@johnnylam88
Copy link
Contributor

@Hemario I think this is fixed in Ovale 9.0.34. Can you verify?

@Sidoine Sidoine closed this as completed Jan 6, 2021
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