azerothcore-wotlk
azerothcore-wotlk copied to clipboard
azerothcore
Implement visibility notifier
Changes Proposed:
- Implement missing visibility notifier functionality.
- Increased default visibility distance for continents, instances and BGs/Arenas
In general more alignment with TrinityCore 3.3.5a master branch.
Issues Addressed:
- Closes https://github.com/azerothcore/azerothcore-wotlk/issues/14074
SOURCE:
AzerothCore current master branch i.e. no visibility notifier: https://www.youtube.com/watch?v=uAkNyw_DK5c
AzerothCore with visibility notifier implemented: https://www.youtube.com/watch?v=VzomyFJECpU
Tests Performed:
- Builds.
- Tested in-game that specific objects and npcs can be seen from a longer distance. See video above.
- Tested on win10
How to Test the Changes:
One can compare visibility distance of objects and npcs on master branch with this branch. A good place to test would be wintergrasp. Note: One might want to adjust their visibility distance configuration in the worldserver.conf
Known Issues and TODO List:
- TODO: Add INTERVAL_GRIDCLEAN to config files and read values from there instead. I wasn't quite sure if the order of the WorldIntConfigs enum matters and also in general if there is anything else to take into consideration when adding new configs.
- Known "issue": Several npcs (and perhaps objects too) have their visibilityDistanceType in creature_template_addon overriden in creature_addon with the wrong value. An example is creature by name "Forgotten Depths Underking" who has their visibilityDistanceType set to 3 (large) in creature_template_addon, but for each creature by their guid it is set to 0 in creature_addon.
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 on the PR's page on GitHub. Follow the instructions here:
http://www.azerothcore.org/wiki/How-to-test-a-PR
REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).
For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.
CPU usage will be about twice as high
since defaults are changed a increase in resource usuage is bound to happen and is expected.
I accurately tested the CPU usage again
MapUpdate.Threads = 16
Five minutes of server startup, no players online
1% -3% before PR
13% -18% after PR
what's the status of this PR?
I have tested and haven't found any errors yet, it's just that the CPU usage is high
Been AFK the last couple of months which is why there has not been any activity, sorry about that 😛
Gave this a test. Everything I can think of seems to work well. One case I am concerned about is that of the fel reavers. They have a VisibilityDistanceType of 5 (infinite supposedly) but they very much aren't infinitely viewable. From what I can recall, they're supposed to be visible from just about the entire zone of Hellfire Peninsula, but a fel reaver placed at Honor Hold isn't visible from Thrallmar. Not sure if this is a case of poor memory on my end or something that may need to be dealt with in this PR or in the future. Again, all else looks well though, excellent work! 👍
@AGandrup could you look at this case? If we get that resolved it is enough for me to merge it
Sure, I'll check it out, however I am out of town the entire week, so will have to wait till I get back home.
Did you check the visibility distance of Fel Reaver on TrinityCore if it is any different?
Also you could check if Fel Reaver has its visibilityDistanceType set to 5 in both creature_addon and creature_template_addon
creature_addon
| guid | path_id | mount | bytes1 | bytes2 | emote | visibilityDistanceType | auras |
|---|---|---|---|---|---|---|---|
| 67001 | 670010 | 0 | 0 | 1 | 0 | 0 | |
| 203341 | 2033410 | 0 | 0 | 1 | 0 | 0 |
@heyitsbench can you try changing visibilityDistanceType to 5? I believe this should fix the issue (Also not related, but TC has auras "19818 34623", do you have a sniff for it, if so we can correct it as well)
The following video is from october 2nd 2008: https://www.youtube.com/watch?v=W_exXtbATr4 I can't tell if this guys graphic settings are just really low or if the draw distance for large creatures were increased later.
EDIT: But then again, this video were made like a month before the release of WotLK, so I guess it aint relevant.
@AGandrup Would you like to test the case above once more, and do a general test? If all looks good we can merge it.
@Kitzunu The max view distance is set to GRID_SIZE (533.3333yd), so that is currently the the limit which is the same as for TC. If you wish to see beyond that it would require some rework of visibility distance calculations as mentioned here https://github.com/TrinityCore/TrinityCore/issues/22986 and, to my understanding, rework of how grids behave and whatnot - see these comments here, for example: https://github.com/TrinityCore/TrinityCore/commit/b0db728c49f0b70d2c4f18270c389e4445161b7f As much as I would like to see this done as well, I don't think its a small task and I dont have the knowledge nor expertise to complete such task right now - at least not alone. But nonetheless I believe that this PR is the first step towards completing that task. So in my opinion, I think a new issue should be created regarding this next step.
@AGandrup This is causing some form of freezing. I am looking at it too but would be nice if you could take a look as you know the system from working on it. Here's a stacktrace https://gist.github.com/Nyeriah/84748d422ed2b4fca35b4e746b04a10d
@AGandrup This is causing some form of freezing. I am looking at it too but would be nice if you could take a look as you know the system from working on it. Here's a stacktrace https://gist.github.com/Nyeriah/84748d422ed2b4fca35b4e746b04a10d
I have not had much time this weekend, so I only had a brief look at this issue as well as the CPU issue mentioned here: https://github.com/azerothcore/azerothcore-wotlk/issues/17439#issuecomment-1751685446
I'll look more into it again later after work, but I don't know how quickly I will be able to find the cause let alone fix it. Unless there are others looking into this, it may perhaps be best to revert the changes for now?
There’s also an issue with doors and other game object not loading up in time that has also been caused by this PR
There’s also an issue with doors and other game object not loading up in time that has also been caused by this PR
Can you provide an example so I can investigate?
https://github.com/azerothcore/azerothcore-wotlk/issues/17432#issuecomment-1751572029
Status: still investigating this issue, haven't found the cause yet however making small steps. Remember when I swapped time_t for std::chrono::seconds? I am starting to think that it should've been miliseconds instead lol. That could potentially affect both issues (for the worse or better, need more testing).
Status: I am pretty confident that I have found whats causing the issue with gameobjects not showing up on time https://github.com/azerothcore/azerothcore-wotlk/issues/17432 Will make a PR shortly. @Kitzunu @Nyeriah
Status updates will continue here: https://github.com/azerothcore/azerothcore-wotlk/pull/17480