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

Run constraint.GetAllConstrainedEntities after timer #337

Merged
merged 4 commits into from
Jul 25, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions lua/fpp/server/ownability.lua
Original file line number Diff line number Diff line change
Expand Up @@ -499,11 +499,14 @@ function FPP.RecalculateConstrainedEntities(players, entities)
end

local entMem = {}
local function constraintRemovedTimer(ent1, ent2, constrainedEnts)
if not IsValid(ent1) and not IsValid(ent2) or not constrainedEnts then return end

FPP.RecalculateConstrainedEntities(player.GetAll(), constrainedEnts)
local function constraintRemovedTimer(ent1, ent2)
if not IsValid(ent1) and not IsValid(ent2) then return end
entMem = {}

local allConstrainedEnts = {}
table.Add(allConstrainedEnts, constraint.GetAllConstrainedEntities(ent1))
table.Add(allConstrainedEnts, constraint.GetAllConstrainedEntities(ent2))
FPP.RecalculateConstrainedEntities(player.GetAll(), allConstrainedEnts)
end

local function handleConstraintRemoved(ent)
Expand All @@ -516,10 +519,7 @@ local function handleConstraintRemoved(ent)
entMem[ent1] = true
entMem[ent2] = true
Copy link
Owner

@FPtje FPtje Jul 21, 2024

Choose a reason for hiding this comment

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

What if we use this entMem: instead of just storing the two entities, what about storing ALL the constrained entities in entMem?

That way, instead of calling the expensive function once (or twice) per unique pair of entities, it would call it once per contraption.

I got this idea from a quick look. Would it work?

Copy link
Contributor Author

@wrefgtzweve wrefgtzweve Jul 24, 2024

Choose a reason for hiding this comment

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

I don't think this would work as entMem is checked before the timer is created.
Seems like the timer here is buggy though as it creates a unique timer that gets overwritten each time the function runs likely losing out on a lot of constraintRemovedTimer calls which should be made.

The current code has pretty much no lag impact even with calling it twice (tested with some workshop dupes with 300+ constraints and 300+ props) so performance should be fine as far as i can see. I did merge the tables to prevent double calling it.

Copy link
Owner

Choose a reason for hiding this comment

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

The profile says more than intuition. Thanks for taking a look 👍


-- the constraint is still there, so this includes ent2's constraints
local constrainedEnts = constraint.GetAllConstrainedEntities(ent1)

timer.Create("FPP_ConstraintRemovedTimer", 0, 1, function() constraintRemovedTimer(ent1, ent2, constrainedEnts) end)
timer.Create("FPP_ConstraintRemovedTimer", 0, 1, function() constraintRemovedTimer(ent1, ent2) end)
end

local function onEntityRemoved(ent)
Expand Down