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

fix #541 : remove weak hook using #2478

Merged
merged 5 commits into from Aug 2, 2020
Merged

fix #541 : remove weak hook using #2478

merged 5 commits into from Aug 2, 2020

Conversation

nheir
Copy link
Contributor

@nheir nheir commented Feb 24, 2020

Use SaturateAdd after summing all the dragging velocity

@nheir nheir changed the title remove weak hook using (fix #541) fix #541 : remove weak hook using Feb 24, 2020
@heinrich5991
Copy link
Contributor

Please use descriptive variable names. InteractVel does not specify that only hook velocity deltas go in there, IgnoreInteractVel does not specify that it is only going to be ignored for the next tick.

CCharacterCore::Move is not the appropriate place for changing the velocity for things that happened in the tick. It should only move the tee.

This change can only be merged in the next major version bump as this changes network code.

@fokkonaut
Copy link
Contributor

I'd rename the function to AddHookDragVel, and I'd also reset the var in this function, just at the end. So you could remove the reset function

@fokkonaut
Copy link
Contributor

And why dont we need it while firing ninja? was it like that before?

@nheir
Copy link
Contributor Author

nheir commented Feb 25, 2020

I'd rename the function to AddHookDragVel, and I'd also reset the var in this function, just at the end. So you could remove the reset function

I used the Reset function to set this additional velocity to whatever the weapon.

And why dont we need it while firing ninja? was it like that before?

I noted that the function HandleWeapon->HandleNinja changes the position of the Tee itself, overriding the velocity so that client doesn't predict stuff.

m_Core.m_Vel = m_Ninja.m_ActivationDir * g_pData->m_Weapons.m_Ninja.m_Velocity;
vec2 OldPos = m_Pos;
GameServer()->Collision()->MoveBox(&m_Core.m_Pos, &m_Core.m_Vel, vec2(GetProximityRadius(), GetProximityRadius()), 0.f);
// reset velocity so the client doesn't predict stuff
m_Core.m_Vel = vec2(0.f, 0.f);

Since this is taking account in Tick, depending on the character order, the players' hook have either no effect or a small effect on the speed and the position.
I chose no effect, since it's easy to make it fair, but I don't claim it is the good choice.
Is the trajectory of a ninja only depending on the initial position and direction ? or do/should other entities and hooks have an effect on this trajectory ?

Since the position is also updated by HandleNinja, there is still a difference when hooking a moving ninja.

And I realize that the players hit by a ninja isn't fair either.

@heinrich5991
Copy link
Contributor

It looks like the hook code is only in the server side now. Have you tested hook interactions in the client side? The code is also used there (which is why it's a protocol change AFAICT).

@oy
Copy link
Member

oy commented Mar 11, 2020

The current prediction is broken anyway. The order of the character handling differs between client and server.
So don't see a problem with adding without a major release.

@oy oy merged commit ca5cb2a into teeworlds:master Aug 2, 2020
@oy oy mentioned this pull request Aug 2, 2020
@ChillerDragon
Copy link
Contributor

yikes

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.

None yet

5 participants