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

HUD: Fixed OHI Desync #1513

Merged
merged 9 commits into from
May 19, 2024
Merged

HUD: Fixed OHI Desync #1513

merged 9 commits into from
May 19, 2024

Conversation

TimGoll
Copy link
Member

@TimGoll TimGoll commented May 13, 2024

Sometimes overhead icons stayed stuck in the world. This is due to the way GMod/source handles player positions. If a player is far enough away, their position is no longer updated (as can be seen with wallhacks for example).

While :GetPos() still returns a position that at least represents a rough estimate of their position, the bone position stays at the last updated place. This means that the overhead icon stays floating in random places.

This pullrequest fixes this by introducing a check that uses the fallback position if the player position and the bone position diverge too much.

While I tested this, it would probably be good if @mexikoedi could confirm this as well.

@TimGoll TimGoll added the type/bug Something isn't working label May 13, 2024
@TimGoll TimGoll added this to the v0.13.3b milestone May 13, 2024
Copy link
Member

@Histalek Histalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While writing my review i'm starting to question if this could really fix this problem. plymeta:GetHeightVector implies to only return height (a vector with a non-nil Z value and a nil x and y value). But the problem in the first place is happening on all 3 axis, right?

While this change might still fix issues on the Z-axis, the X- and Y-axis issues can't really be fixed by this.

That is at least what i get from this. Am i missing something?

gamemodes/terrortown/gamemode/shared/sh_player_ext.lua Outdated Show resolved Hide resolved
gamemodes/terrortown/gamemode/shared/sh_player_ext.lua Outdated Show resolved Hide resolved
@TimGoll
Copy link
Member Author

TimGoll commented May 13, 2024

While writing my review i'm starting to question if this could really fix this problem. plymeta:GetHeightVector implies to only return height (a vector with a non-nil Z value and a nil x and y value). But the problem in the first place is happening on all 3 axis, right?

While this change might still fix issues on the Z-axis, the X- and Y-axis issues can't really be fixed by this.

That is at least what i get from this. Am i missing something?

I thought you were right, but this is a bit more tricky. My intention was to return a height vecvor, yes. But when the positions between the headbone and the player position diverge, x and y offsets are introduced in the height vector:

return pos - posPlayer

In general I'd like to keep it that way so that it tracks the position and puts this movement in the x and y offset. So maybe the function name is misleading.

The introduced change catches the divergence though, so my fix should work.

TimGoll and others added 2 commits May 13, 2024 23:38
Co-authored-by: Histalek <16392835+Histalek@users.noreply.github.com>
@TimGoll
Copy link
Member Author

TimGoll commented May 13, 2024

@Histalek I applied your suggestions and also renamed a variable to make it more clear. I'm debating whether I should rename the function. Right now it is called GetHeightVector(). Which is only kinda true because it also tracks the X and Y deviation of the head position in relation the the player position. Maybe it should be called GetHeadPosition or something similar?

@homonovus
Copy link
Contributor

the player's position not updating is due to entity dormancy on the client; you can check this for an early return in rendering using entity:IsDormant()

an entity is dormant when it is not in a player's PVS, and when so, the entity will net send updates about its state to players who it is dormant for

for players, an exception is made in GMod, where they are transmitted for a single tick every 10 seconds

@Histalek
Copy link
Member

While writing my review i'm starting to question if this could really fix this problem. plymeta:GetHeightVector implies to only return height (a vector with a non-nil Z value and a nil x and y value). But the problem in the first place is happening on all 3 axis, right?
While this change might still fix issues on the Z-axis, the X- and Y-axis issues can't really be fixed by this.
That is at least what i get from this. Am i missing something?

I thought you were right, but this is a bit more tricky. My intention was to return a height vecvor, yes. But when the positions between the headbone and the player position diverge, x and y offsets are introduced in the height vector:

return pos - posPlayer

In general I'd like to keep it that way so that it tracks the position and puts this movement in the x and y offset. So maybe the function name is misleading.

definitely misleading imo, but since it already was like this before your changes i don't think we need to address this here.

The introduced change catches the divergence though, so my fix should work.

Makes sense

@Histalek
Copy link
Member

@Histalek I applied your suggestions and also renamed a variable to make it more clear.

Thanks!

I'm debating whether I should rename the function. Right now it is called GetHeightVector(). Which is only kinda true because it also tracks the X and Y deviation of the head position in relation the the player position. Maybe it should be called GetHeadPosition or something similar?

GetHeadPosition sounds good! although i wouldn't want to rename the function right now as i have no idea who might be using this and they might just rely on this unexpected behaviour.

For now leaving this as is should be the lesser evil imo

Copy link
Member

@Histalek Histalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as it is

you could look into the entity:IsDormant() that @homonovus mentioned, maybe that could lead to an even nicer if-condition :)

@TimGoll
Copy link
Member Author

TimGoll commented May 14, 2024

okay, I will use entity:IsDorman(). I wasn't aware that there is a function to check that. Therefore my hacky approach. Thank you @homonovus!

@Histalek about the name: I introduced this function in the last minor release - so I guess it should be fine to rename. Nobody is using it

@mexikoedi
Copy link
Contributor

I tested a bit around and with different distances too. I couldn't reproduce this issue anymore. So I think it's fixed or I had luck because it's a bit difficult to reproduce.
The only thing I noticed was that the OHI is stuck if you go outside the map but I think that's a different topic because other things (player name, ...) get also stuck.

Is a similar fix applicable to this addon: https://steamcommunity.com/sharedfiles/filedetails/?id=1137493106
Because this has the issue which was described in the description.

@TimGoll
Copy link
Member Author

TimGoll commented May 15, 2024

Done and tested

Co-authored-by: Histalek <16392835+Histalek@users.noreply.github.com>
@TimGoll TimGoll merged commit c49529f into master May 19, 2024
4 checks passed
@TimGoll TimGoll deleted the fix-ohi-desync branch May 19, 2024 13:51
@Histalek Histalek modified the milestones: v0.13.3b, v0.14.0b Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants