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.

Sign up for GitHub

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

Jump to bottom

feat(Core/Battlegrounds) Reworked enhanced bg system for modules #2521

Merged
merged 5 commits into from
Jan 19, 2020
Merged

feat(Core/Battlegrounds) Reworked enhanced bg system for modules #2521

merged 5 commits into from
Jan 19, 2020

Conversation

Shard-MW
Copy link
Member

Reworked #2334
Special thanks to for his main PR @Helias <3

CHANGES PROPOSED:
  • Fixed score errors from base PR (while queuing Random Queue)
  • Fixed worldstate/score errors in our bg-modules that were occurring only from Random Queue
  • Removed non-needed changes
  • Added hashmaps and conditions
TESTS PERFORMED:
  • Joined and played until end every battlegrounds from Random Queue
  • Joined and played until end every battlegrounds from Specific Queue
  • Joined and played until end every arenas (skirmish/rated)
  • Checked score & worldstates everytime
HOW TO TEST THE CHANGES:
  • Join & play every battlegrounds from Random Queue
  • Join & play every battlegrounds from Specific Queue
  • Join & play every arenas (skirmish/rated)
  • Don't forget to check scores panel
Target branch(es):
  • Master

How to test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here in the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

@Helias Helias mentioned this pull request Dec 21, 2019
Closed
@FrancescoBorzi
Copy link
Contributor

@Endeffect-AT can you please have a look at this?

@LukasVolgger
Copy link
Member

LukasVolgger commented Jan 9, 2020 •
edited
Loading

Will test this and PR #1903 the next days, just need a little time

Viste
Viste suggested changes Jan 9, 2020
View reviewed changes
src/server/game/Battlegrounds/BattlegroundMgr.cpp Show resolved Hide resolved
src/server/game/Battlegrounds/BattlegroundMgr.cpp Outdated Show resolved Hide resolved
src/server/game/Battlegrounds/BattlegroundMgr.cpp Outdated Show resolved Hide resolved
src/server/game/Battlegrounds/BattlegroundMgr.cpp Outdated Show resolved Hide resolved
src/server/game/Entities/GameObject/GameObject.cpp Outdated Show resolved Hide resolved
src/server/game/Entities/GameObject/GameObject.cpp Outdated Show resolved Hide resolved
src/server/game/Entities/Player/Player.cpp Outdated Show resolved Hide resolved
src/server/game/Entities/Player/Player.cpp Outdated Show resolved Hide resolved
@LukasVolgger
Copy link
Member

I guess it's wip and i test it when ready 👍

@Shard-MW
Copy link
Member Author

Shard-MW commented Jan 16, 2020 •
edited
Loading

@Endeffect-AT

I guess it's wip and i test it when ready 👍

It's not WIP, it's a finished PR haha

@LukasVolgger
Copy link
Member

I refer to the requested changes from @Viste

@LukasVolgger
Copy link
Member

LukasVolgger commented Jan 18, 2020 •
edited
Loading

Tested on win 10 pro 64 bit, AC commit 70170f0.
Ran build without any errors, no crashes during the performed tests. Every bg was tested with .debug bg and one pleayer each faction.

Note: Didn't check the worldstates.

P.S: ~ 4h Testing time

Battlegrounds

Joined & played every BG until the end from Random Queue:

  • Isle of Conquest

Scoreboard Error: Joined with one pleayer each faction, 2 Horde are shown.

image

  • Eye of the Storm

Statistics work as expected

  • Alterac Valley

Statistics work as expected

  • Strand of the Ancients

Statistics work as expected

  • Warsong Gulch

Statistics work as expected

  • Arathi Basin

Statistics work as expected

  • Farm Horde Banner Bug: As horde char you didn't see and can't interactive with the banner. Only as the Alliance has taken it, you could'd see and interact with it.

image

image

Joined & played every BG until the end from Specific Queue:

  • Isle of Conquest

Statistics work as expected

  • Eye of the Storm

Statistics work as expected

  • Alterac Valley

Statistics work as expected

  • Strand of the Ancients

Statistics work as expected

  • Warsong Gulch

Statistics work as expected

  • Arathi Basin

Statistics work as expected

Arenas rated (2v2)

Team with ingame petition signed and joined every arena:

  • Nagrand Arena

Statistics work as expected

  • Ring of Valor

Statistics work as expected

  • Dalaran Arena

Statistics work as expected

  • Ruins of Lordaeron

Statistics work as expected

  • Circle of Blood Arena

Statistics work as expected

Arenas skirmish (2v2)

Joined every arena in group:

  • Ring of Valor

Statistics work as expected

  • Circle of Blood Arena

Statistics work as expected

  • Ruins of Lordaeron

Statistics work as expected

  • Nagrand Arena

Statistics work as expected

  • Dalaran Arena

Statistics work as expected

ToDo - List:

- [ ] Fix Scoreboard in BG "Isle of Conquest" from Random Queue
- [ ] Fix Horde Farm Banner in "Arathi Basin" from Random Queue

Others:

  • Wintergrasp (Didn't played until end, just joined to test)

If you join Wintergrasp on both sides in Dalaran at the Battlemaster an click on minimap "Leave Wintergrasp" you got teleported at the starting zone of your race. I don't think this should happen....

@Shard-MW
Copy link
Member Author

Shard-MW commented Jan 18, 2020 •
edited
Loading

Wow. Thanks a lot for taking that much time for testing, you're awesome @Endeffect-AT ! <3

It's weird that only "Isle of Conquest" have an issue with scoreboard, since this PR shouldn't touch at this... Can you try if you've the same error without the PR please ?

About

Fix Horde Farm Banner in "Arathi Basin" from Random Queue

This is not triggered by the PR. It's a random bug that AC have, I've reported this before :
#2518

@FrancescoBorzi
Copy link
Contributor

@Endeffect-AT great job! What a nice report.

I don't think that any of the bugs that you mentioned have been introduced by this PR.

For example, the Arathi Basin missing flag is an ancient bug that randomly happens and I usually clean my client cache to fix it

@LukasVolgger
Copy link
Member

LukasVolgger commented Jan 18, 2020 •
edited
Loading

Thx guys <3

I noticed the bug with the banner the first time on testing this: #1903

I thought it was caused by me with GM Mode. I tried it again and it worked. Okay so this is not related to this PR!

I will test the Scoreboard in Isle of Conquest on master soon @Shard-MW!

@LukasVolgger
Copy link
Member

Tested Isle of Conquest from Random Queue on master commit 0f590d0+ (Yes, i should update my core...) and the bug is the same.

image

So your PR is working fine imo 👍

@Shard-MW
Copy link
Member Author

@Endeffect-AT
Thanks again ! I think you can open an issue about Isle of Conquest so :P

@FrancescoBorzi
Copy link
Contributor

@Endeffect-AT cool, thanks for checking that too. And good job @Shard-MW

@Viste
Copy link
Contributor

Viste commented Jan 19, 2020

plz fix codestyle.

@Shard-MW
Copy link
Member Author

plz fix codestyle.

Done, but I'll not change this :
https://github.com/azerothcore/azerothcore-wotlk/pull/2521/files#diff-85ba4e80476ac43de064d54592507596R1105
because it looks horrible since it's not a function

@FrancescoBorzi FrancescoBorzi merged commit eadcb1a into azerothcore:master Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Viste Viste Viste requested changes

@Helias Helias Helias approved these changes

@vhiperdev vhiperdev vhiperdev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Shard-MW @FrancescoBorzi @LukasVolgger @Viste @Helias @vhiperdev