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

onPickupLeave+setDimension #846

Open
Allerek opened this issue Mar 6, 2019 · 8 comments
Open

onPickupLeave+setDimension #846

Allerek opened this issue Mar 6, 2019 · 8 comments
Labels
bug Something isn't working
Milestone

Comments

@Allerek
Copy link
Contributor

Allerek commented Mar 6, 2019

onPickupLeave do not detect the dimension change,idk how about interior.

@botder
Copy link
Member

botder commented Mar 6, 2019

Can you use the Bug report issue template next time? Your description is lacking information.
How to reproduce your bug? What is the expected behavior? What is your server/client version?

@botder botder added the bug Something isn't working label Mar 6, 2019
@botder botder added this to the Backlog milestone Mar 6, 2019
@Allerek
Copy link
Contributor Author

Allerek commented Mar 6, 2019

Can you use the Bug report issue template next time? Your description is lacking information.
How to reproduce your bug? What is the expected behavior? What is your server/client version?

  1. Create pickup
  2. Stand in it, then teleport to other dimension.
    I have the script that makes GUI appear when you hit Pickup the triggerClientEvent with function when you leave the pickup thats hiding the GUI, and it doesnt hide if you change dimension when standing in pickup.
    Client/Server version-Latest Public

@Allerek
Copy link
Contributor Author

Allerek commented Mar 6, 2019

@Allerek Allerek closed this as completed Mar 6, 2019
@Allerek Allerek reopened this Mar 6, 2019
@botder
Copy link
Member

botder commented Mar 6, 2019

Client

void CClientPickup::Callback_OnLeave(CClientColShape& Shape, CClientEntity& Entity)
{
if (IS_PLAYER(&Entity))
{
bool bMatchingDimensions = (GetDimension() == Entity.GetDimension()); // Matching dimensions?
// Call the pickup leave event (source = the pickup that was left)
CLuaArguments Arguments;
Arguments.PushElement(&Entity); // The element that left the pickup
Arguments.PushBoolean(bMatchingDimensions);
CallEvent("onClientPickupLeave", Arguments, true);
// Call the player pickup leave event (source = the player that left the pickup)
CLuaArguments Arguments2;
Arguments2.PushElement(this); // The pickup that was left (this)
Arguments2.PushBoolean(bMatchingDimensions);
Entity.CallEvent("onClientPlayerPickupLeave", Arguments2, true);
}
}

Server

void CPickup::Callback_OnLeave(CColShape& Shape, CElement& Element)
{
if (IS_PLAYER(&Element))
{
CPlayer& Player = static_cast<CPlayer&>(Element);
// Matching interior
if (GetInterior() == Element.GetInterior())
{
// Matching dimension
if (GetDimension() == Element.GetDimension())
{
// Is he alive?
if (!Player.IsDead())
{
// Call the onPickupLeave event
CLuaArguments Arguments;
Arguments.PushElement(&Player);
CallEvent("onPickupLeave", Arguments);
CLuaArguments Arguments2;
Arguments2.PushElement(this); // pickup
Element.CallEvent("onPlayerPickupLeave", Arguments2);
}
}
}
}
}

Proposed solution

Change CPickup::Callback_OnLeave (server) to match the behavior of CClientPickup::Callback_OnLeave (client):

  • Add matchingDimensions parameter
  • Trigger for leaving players in different interior and dimension

We could also do the same for CPickup::Callback_OnCollision (client triggers onClientPickupHit even if interior and dimension differs, but server doesn't), but it might break existing scripts.

@botder botder added the good first issue Good for newcomers label Mar 6, 2019
@qaisjp
Copy link
Contributor

qaisjp commented Apr 14, 2019

Actually I think it's good that onPickupLeave server-side currently only triggers for matching interiors and dimensions.

I am not sure what to do if the user leaves a pickup by teleporting to another dimension. It should probably trigger the event even though the dimension/interior is different, as the player has technically left the pickup.

I have a hunch that the clientside argument was only added in the name of backwards compatibility

Thoughts @botder?

@botder
Copy link
Member

botder commented Apr 15, 2019

I really dislike keeping backwards compatibility in such cases, because the next scripter might not expect this odd behavior - he probably expects that leaving the pickup's dimension/interior should trigger the event.

@qaisjp
Copy link
Contributor

qaisjp commented Apr 16, 2019

  • Add matchingDimensions parameter [the first half]
  • Trigger for leaving players in different interior and dimension [the second half]

I really dislike keeping backwards compatibility in such cases, because the next scripter might not expect this odd behavior - he probably expects that leaving the pickup's dimension/interior should trigger the event.

I agree we should add a fix for the second half. My backwards compatibility comment is only relating to the first half.

I have a hunch that the clientside argument was only added in the name of backwards compatibility

This meant that the client-side event probably originally used to always trigger (regardless of dimension and interior). So they added matchingDimension for people to check instead of breaking BC a little.

My argument is that it is good that onPickupLeave does not trigger when you pickup is in a different dimension. i.e., we should not remove the dimension/interior checks on the server side.

Also, on the client, matchingDimensions should probably be true if the player leaves the pickup's dimension, since the player USED TO BE in the same dimension.

Does that make sense?

@Allerek
Copy link
Contributor Author

Allerek commented Nov 29, 2020

Refresh, is it fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants