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

OutdoorPVP SI/EP #571

Closed
wants to merge 1 commit into from
Closed

OutdoorPVP SI/EP #571

wants to merge 1 commit into from

Conversation

Hectelion
Copy link

entering silithus/eastern plaguelands when outdoorpvp is not enabled will show no longer the AlwaysUpFrame* on top of the screen

🍰 Pullrequest

Avoid the AlwaysUpFrame* showing up when the OutdoorPVP is set to not enable in mangosd.conf

Proof

Issues

doing the teleport to SI/EP without this update may often cause the AlwaysUpFrames show up even when outdoorpvp is not enable

How2Test

  • without patch: set both the OutdoorPvp.*Enabled = 0, teleport to SI or EP and see the AlwaysUpFrames on top of the screen
  • with patch: set both the OutdoorPvp.*Enabled = 0, teleport to SI or EP and see the AlwaysUpFrames are not showed

Todo / Checklist

  • None

entering silithus/eastern plaguelands when outdoorpvp is not enabled will show no longer the AlwaysUpFrame* on top of the screen
@insunaa
Copy link
Contributor

insunaa commented Jul 14, 2024

Please provide proof that the worldstate panel should be invisible in the area when PVP is disabled. Also for the love of all the gods don't do things like

if (someCondition)
  {}
else
  someStuff;

@Hectelion
Copy link
Author

Immagine 2024-07-14 193447
Immagine 2024-07-14 193520
Immagine 2024-07-14 193651
Immagine 2024-07-14 193721
added some screenshots; about the code any suggestion to do it better is welcome

@insunaa
Copy link
Contributor

insunaa commented Jul 14, 2024

I meant proof for how it's supposed to be on ABK servers. I believe you when you say that those are the changes you made, but the question is if those changes are legitimate in the first place.

As for the code, you'll have to rearrange the condition so that you can do

if (someCondition)
  someStuff;

@Exxenoz
Copy link
Member

Exxenoz commented Jul 14, 2024

In your PR, no world states are sent at all for the two zones when opvp is disabled. It would be better to take a more targeted approach here. There is probably a world state or opcode for showing/hiding this specific display.

@insunaa
Copy link
Contributor

insunaa commented Jul 14, 2024

That assumes that the world state display is actually meant to be hidden. I suspect that since the World-PVP Event in those zones is never disabled on live servers and was never meant to be disabled either, that there's no dedicated functionality to actually hiding the worldstate display.
But I don't know so I'd like to see proof that this change accurately reflects the reference environment.

@Exxenoz
Copy link
Member

Exxenoz commented Jul 14, 2024

That assumes that the world state display is actually meant to be hidden. I suspect that since the World-PVP Event in those zones is never disabled on live servers and was never meant to be disabled either, that there's no dedicated functionality to actually hiding the worldstate display. But I don't know so I'd like to see proof that this change accurately reflects the reference environment.

Seems like display activation is bound to SMSG_INIT_WORLD_STATES and its zone id. The display is even activated when an empty (without any world state data) SMSG_INIT_WORLD_STATES is sent.

killerwife added a commit to cmangos/mangos-tbc that referenced this pull request Nov 13, 2024
killerwife added a commit to cmangos/mangos-wotlk that referenced this pull request Nov 15, 2024
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