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

Continous Use: Reimplemented #1645

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Continous Use: Reimplemented #1645

wants to merge 2 commits into from

Conversation

TimGoll
Copy link
Member

@TimGoll TimGoll commented Sep 28, 2024

Fixes #1636

Due to the use rework, continuous use was broken. This PR fixes it by adding a timer that is called as long as the use key is pressed and the entity is close by.
Most of these changes are just code being moved into another function, the timer part is new though

@TimGoll TimGoll added the type/bug Something isn't working label Sep 28, 2024
@TimGoll TimGoll added this to the v0.14.1b milestone Sep 28, 2024
@ZenBre4ker
Copy link
Contributor

Not sure that all those actions were continously useable before my rework.
Spec, Bodysearch etc.

Also I'd normally have said, that those actions are triggered every frame. 0.1s seems a bit weird. But I am not sure how the timing was before, but it should probably be tickrate dependent. And every implementation of continuos use should do some deltaTime calcs to make sure the amount is tickrate independent.

@TimGoll
Copy link
Member Author

TimGoll commented Sep 28, 2024

Not sure that all those actions were continously useable before my rework.
Spec, Bodysearch etc.

Some of them are exited early, so are map entities withou continous use. I'm not sure how I could handle the rest, but I guess it is identical to how it was before? If you have any ideas, go ahead.

And yes, I could change to timer ot every tick (0 time) as well. I wasn't sure what the best interval here would be

@TimGoll
Copy link
Member Author

TimGoll commented Sep 29, 2024

@ZenBre4ker Judging by this:

https://github.com/ValveSoftware/source-sdk-2013/blob/master/sp/src/game/shared/baseplayer_shared.cpp#L161-L195

and this:

https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/sp/src/game/shared/baseplayer_shared.cpp#L1275-L1399

I think it is safe to assume that it was handled every frame before.

Now I'm wondering if a 0 tick timer would be a good solution here. Or if I should handle it in the Think hook as well. ItemPreFrame is called in PreThink.

@ZenBre4ker
Copy link
Contributor

0 tick timer should suffice. But I will take a look at the continuous use for the other interactions. Feel free to merge this if you tested it, as I wont promise to come up with an analysis early.

The following idea is meant as an enhancement and therefore should not be taken into consideration for this MR:

Im actually thinking about reverting it a little, so to return true on serverside use again.
On the server you allow use on clientside sent entities, but always prefer the server's view. If the client sees one, but the server doesnt, use the client one (our rework). If the server sees one, but the client doesnt, use the server ones (old behaviour). If both see different ones, we could prefer the server's view (old behaviour). This would at least help with entities the client can see, but the server has in another position and the other way round as well with the weird teleport button on waterworld or cars that dont behave well with our rework.

This would bring the old behavior back in most cases and only add the feature that the client can argue that there is an entity to use. To not double activate use, we could cache used ents to see if Use was triggered on the server already. It could be complicated though, therefore its just an idea.

@TimGoll
Copy link
Member Author

TimGoll commented Sep 29, 2024

I tested it and put it into today's release candidate. We will see what the feedback is

Copy link
Contributor

@ZenBre4ker ZenBre4ker left a comment

Choose a reason for hiding this comment

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

Continuous use should be every tick and it should be tested on bodies, specs and all entities that are triggered in the timer. Then I'd approve.

Comment on lines +93 to +95
if self:IsWeapon() or self.player_ragdoll then
return true
elseif self:IsScripted() and self.CanUseKey then
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if self:IsWeapon() or self.player_ragdoll then
return true
elseif self:IsScripted() and self.CanUseKey then
if self:IsWeapon() or self.player_ragdoll or (self:IsScripted() and self.CanUseKey) then

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure anymore if the brackets are needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I split this into two because I thought it is more readable

Copy link
Member

Choose a reason for hiding this comment

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

i second zen's approach. 2 ifs are more confusing IMO

also the parenthesis should not be needed

end

-- make sure it is called 10 times per second
timer.Simple(0.1, function()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be 0 IMO

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, but I have to test this to see if that works

Copy link
Member

@Histalek Histalek Nov 27, 2024

Choose a reason for hiding this comment

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

This sadly does not work

Edit: setting the timer to 0 i mean; 0.1 instead works flawlessly

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.

Use Interactions: Healthstation Broken
3 participants