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

Pet Frame not updating unit correctly #257

Closed
ghost opened this issue Sep 30, 2015 · 13 comments
Closed

Pet Frame not updating unit correctly #257

ghost opened this issue Sep 30, 2015 · 13 comments
Assignees

Comments

@ghost
Copy link

ghost commented Sep 30, 2015

Not sure if anyone keeps this updated anymore bute here we go:

When you're in a vehicle and the player jumps onto the petframe, and when you then exit again the player frame get stuck in the pet frame. (2 player frames).

Deleting this line fixed it, but I'm sure it's there for a reason:
https://github.com/haste/oUF/blob/master/ouf.lua#L54

@ghost ghost mentioned this issue Oct 1, 2015
@haste
Copy link
Member

haste commented Nov 15, 2015

Is there a place I can test this?

@Pondidum
Copy link
Contributor

@haste I found the easiest place to test this is on the tillers farm with one of the weeds which you can yank up or get knocked around by.

Another option are the vehicles at the beginning of Ulduar.

@p3lim
Copy link
Member

p3lim commented Jul 16, 2016

I can't replicate this, tried with and without a pet frame, with and without a pet, at Tillers farm, Ulduar and Argent Tournament.

All works fine.

@ls-
Copy link
Member

ls- commented Jul 17, 2016

@p3lim, I created a frost mage, went to Icecrown -> Argent Tournament, summoned my elemental, took a spear, got on horse, attacked Melee Target and dismounted, while in combat. My pet frame still displays player info.
http://imgur.com/a/0JqhN
I moved pet frame to bottom right corner, so it's next to tooltip.

Actually, even getting off horse w/o attacking anything produces same results o_O
http://imgur.com/a/XSvhi

@p3lim
Copy link
Member

p3lim commented Jul 17, 2016

I see, got the same result.

@ghost
Copy link
Author

ghost commented Sep 11, 2016

I'll just put some more info here. This is happening because UNIT_EXITED_VEHICLE gives player as arg1 on the petframe when the player leaves the vehicle, which makes sense because it was the player that left the vehicle. This unit (player) doesn't match either the realUnit or modUnit so the active unit on the pet frame does not get updated.

@Rainrider Rainrider self-assigned this Sep 12, 2016
@Rainrider
Copy link
Member

@Sticklord is right about the reason it fails to update the pet frame on UNIT_EXITED_VEHICLE

I have some further questions though, just to be sure I'm not missing something.

In ouf.lua:

if(modUnit == "pet" and realUnit ~= "pet") then
    modUnit = "vehicle"
end

Is this really needed? E.g. UnitHealth("pet") and UnitHealth("vehicle") both return the same values when the player is in a vehicle.

if(not UnitExists(modUnit) or unit and unit ~= realUnit and unit ~= modUnit) then return end

What is the reason behind comparing the event unit with the frame units? Events are either registered by RegisterUnitEvent, so it will be the proper unit, or are unitless, so either there is no unit or the unit is not considered relevant.

@p3lim
Copy link
Member

p3lim commented Sep 12, 2016

@Rainrider Maybe @Adirelle knows something, he/she and @haste were working on that part 5 or so years ago, and then find the relevance with the new unit events.

@haste
Copy link
Member

haste commented Sep 12, 2016

Rewriting pet to vehicle was done because the vehicle unit contained vehicle data before the pet unit did. That part of the code is over seven years old, so the semantics have probably changed a bit since then. :)

ref. c47e66b

@Rainrider
Copy link
Member

I'm also not sure why UNIT_*_VEHICLE events are registered as unitless. I need to find a way to test this in a party/raid environment. This does not seem to be required for player/pet though.

@haste
Copy link
Member

haste commented Sep 12, 2016

1eead35 says it's for them to properly update.

The vehicle system has mostly been the same since it was first introduced, so it really needs a full review. Lots of the code was written to workaround edge cases in the early days.

@Adirelle
Copy link
Contributor

Adirelle commented Sep 13, 2016

IIRC, UNIT_*_VEHICLE events are emitted for "player" not "pet". If you registered these events on the "pet" frame for the "pet" unit, it would never get them. The unitless registration is maybe a bit overkill though. We should test if registering "player" events on the "pet" frame works.

I do not remember how all this works with party/raid vehicles, which were used in Ulduar.

@Rainrider
Copy link
Member

Upon entering a vehicle, the modified unit for the pet frame is "player", so UpdateUnits will register the *_VEHICLE events on the pet frame for both "pet" and "player". That's why I believe unitless is unneeded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants