Core/Player: Prevent Forced UpdateObjectVisibility Before in World - #27914

I encountered this during testing the PR, possibly a regression? But I'm not really sure if it is or not. Let me revert and test to find out: https://i.gyazo.com/4005455bf9415b742af342207e394e94.mp4

is that method used to tell the player to refresh how the objects around look, or is it used to tell other objects around to refresh how the player looks ?

is that method used to tell the player to refresh how the objects around look, or is it used to tell other objects around to refresh how the player looks ?

Afaik it basically takes the player as a reference point (if force it used) and iterates the cells nearby and calls UpdateVisibilityOf on nearby players. Otherwise they're scheduled for relocation notifying.

template<class T>
inline void Trinity::VisibleNotifier::Visit(GridRefManager<T> &m)
{
    for (typename GridRefManager<T>::iterator iter = m.begin(); iter != m.end(); ++iter)
    {
        vis_guids.erase(iter->GetSource()->GetGUID());
        i_player.UpdateVisibilityOf(iter->GetSource(), i_data, i_visibleNow);
    }
}

Are you familiar with this bug or is this a regression https://i.gyazo.com/4005455bf9415b742af342207e394e94.mp4 ? I haven't been able to reproduce it more than once.

Tbh running in place could have been because of breakpoints/debugger. I find it impossible to reproduce it a second time.

I wonder if this can be connected to cmsg_loading_screen_notify, which might be used to inform the server that the client is ready

Sigh...

Sigh...

Ohhh, thank you for linking. I didn't know the issue was reported. That issue as well as "Out of Order Packets on Login" are definitely related! Dang, 2017 is quite a long while ago.

As for cmsg_loading_screen_notify @Ovahlord mentioned I am not sure. That is from a version of WoW I have never worked on.

I wonder if this can be connected to cmsg_loading_screen_notify, which might be used to inform the server that the client is ready

I don't believe such an opcode exists in WoTLK. If I recall right during initial login loading screen there is no client acks, it just waits for server data.

For map changes I did one day accidentally learn it's super important to wait for MSG_MOVE_WORLDPORT_ACK before sending any map object related data.

Is there still some research going on for this PR and should it be changed to draft or is it ready to be merged ?

Is there still some research going on for this PR and should it be changed to draft or is it ready to be merged ?

I consider it complete, nothing left for me to do. Though I've not tested it on live servers I haven't personally encountered any regressions on my fork.

I use nVidia 2080, I thought it was the graphics driver issue,so it is server issue?
and also npc sometimes also shows black. but switch map will show normal.

I use nVidia 2080, I thought it was the graphics driver issue,so it is server issue? and also npc sometimes also shows black. but switch map will show normal.

No dude it never was a gpu or windows issue , but some people just prefer to close valid issues when they doesn't understand what's about.

Is there still some research going on for this PR and should it be changed to draft or is it ready to be merged ?

I have a few users on a TS-WoW based server that could reproduce this often enough to be a nuisance.

I've deployed a build today with these changes applied and will do some rudimentary testing.

So far so good. For the users who could pretty consistently reproduce this it hasn't happened again. Haven't noticed any weird after effects or stability issues.

Alright, I finally had time to look at this and compare code between 3.3.5 and master branches. Turns out master branch already had this fixed over 7 years ago in 3ec88b7

In my opinion that old commit is actually a better fix - we don't want to move those visibility updates after login is completed but skip them entirely. That is because the last step of logging in is Map::AddPlayerToMap, which sends all the neccessary (and up-to-date) data

Alright, I finally had time to look at this and compare code between 3.3.5 and master branches. Turns out master branch already had this fixed over 7 years ago in 3ec88b7

In my opinion that old commit is actually a better fix - we don't want to move those visibility updates after login is completed but skip them entirely. That is because the last step of logging in is Map::AddPlayerToMap, which sends all the neccessary (and up-to-date) data

I was unaware that someone else noticed this years ago. Seems like someone came to a similar conclusion back then but I'm not sure they realized that it fixes afew odd bugs. Either way works, the behavior should be the same. Up to you which version you want in the 3.3.5 branch. I initially wrote it that way too but decided to change the behavior as minimally as possible. Setting visibility/notify bits to be handled later seemed like the minimal change.

38dc566