wiremod / advdupe2

Advanced Duplicator 2
http://wiremod.com
Apache License 2.0
90 stars 60 forks source link

Axis constrained entities spawned by SEnt not behaving correctly after duplication #321

Closed aerofur closed 3 years ago

aerofur commented 3 years ago

What I think is happening:

The Wheels are being spawned at 0,0,0 relative to the Railcar? They were copied at 245.8,0,1.4 and should only spawn there. I have code that removes any extra Wheels it spawns (as they are spawned in ENT:Initialize()), after the duplication has finished, I tested with and without, and its only being caused by AdvDupe2 spawning the Wheels.

Issue does not occur with normal Gmod duplicator.

Expected behaviour:

Result:

Video demonstration: https://imgur.com/a/SoXBYRu

Might be an issue with my code, as far as I am aware, and from my testing, It isnt. The SEnt source code can be found here.

thegrb93 commented 3 years ago

I think this is screwing up the duplicator's constraint location logic https://github.com/TitusStudiosMediaGroup/garrysmod_lua_railcars/blob/main/lua/entities/gmod_railcars_testcar.lua#L65

aerofur commented 3 years ago

I removed the set pos, same issue still occurring, just at a different position (it spawns lower now). It needs the set pos so that the wheels do not spawn in the ground.

thegrb93 commented 3 years ago

Ok I'll try to give it a look this weekend.

aerofur commented 3 years ago

Latest dev build also has the same issue. I removed the old SetPos on the parent, moved to the ENT:SpawnFunction(). Any SetPos is messing with the constraints? Wouldn't explain why simfphys cars work when duped.. unless its only on a few constraint types, unsure.

https://github.com/TitusStudiosMediaGroup/garrysmod_lua_railcars/blob/e66c5c74d0a4c67be19f4ff06e15b9adeadb8502/lua/entities/gmod_railcars_testcar.lua#L46

thegrb93 commented 3 years ago

Is this a new problem? I know @robotboy655 recently changed some slider stuff

aerofur commented 3 years ago

Unsure, I’ve only very recently been experimenting with SEnts. Fascinating

thegrb93 commented 3 years ago

Ah, I don't think he touched Axis though so nevermind.

robotboy655 commented 3 years ago

Yeah, it was sliders and ropes.

thegrb93 commented 3 years ago

@robotboy655 I remember one of my chips had to be updated because you have to wait a frame now after multiple SetPos on the same frame before doing an elastic constraint. I wonder if this is related.

@TitusStudiosMediaGroup Can you try with https://github.com/TitusStudiosMediaGroup/garrysmod_lua_railcars/blob/main/lua/entities/gmod_railcars_testcar.lua#L68-L75 changed to

timer.Simple(0.02,function()
if constraint.CanConstrain(self,0) then
    Bogie1 = ModelCreate(self,"prop_physics",nil,BogieModel,self:LocalToWorld(Bogie1Pos),self:GetAngles(),0,0)
    constraint.Axis(Bogie1,self,0,0,Vector(0,0,0),Vector(0,0,0),0,0,0,1,Vector(0,0,1))
    Bogie2 = ModelCreate(self,"prop_physics",nil,BogieModel,self:LocalToWorld(Bogie2Pos),self:GetAngles(),0,0)
    constraint.Axis(Bogie2,self,0,0,Vector(0,0,0),Vector(0,0,0),0,0,0,1,Vector(0,0,1))
    self:DeleteOnRemove(Bogie1)
    self:DeleteOnRemove(Bogie2)
end
end)

I'm curious if that fixes it. I'm on vacation the rest of this week after I get home so I can try then.

aerofur commented 3 years ago

Yup that fixed it! Thats rather interesting that you have to wait, just added a timer of 0s. Just need to add a entity rego so that I can dupe it with the submaterials. Thanks for the help! Closing now?

thegrb93 commented 3 years ago

No, that's just a hacky work-around. Need to investigate the root of the problem. Glad that worked though. Helps narrow things down.

aerofur commented 3 years ago

Ah right I see I see. Atleast I can keep working on more dupe functions for the time being.

thegrb93 commented 3 years ago

The root of it is probably in here https://github.com/wiremod/advdupe2/blob/master/lua/advdupe2/sv_misc.lua#L20. I'm wondering if the physobj Pos/Ang is needing a frame to update now. Maybe a longer timer would fix it.

thegrb93 commented 3 years ago

This timer https://github.com/wiremod/advdupe2/blob/master/lua/advdupe2/sv_misc.lua#L82

thegrb93 commented 3 years ago

@TitusStudiosMediaGroup can you see if increasing that to 0.2 or 0.3 makes it work without your own work-around? If so then I think it's some kind of desyncing between physobj and entity.

aerofur commented 3 years ago

On it

aerofur commented 3 years ago

Setting it to 0.2 fixed the issue without the workaround.

thegrb93 commented 3 years ago

@robotboy655 I think this was introduced sometime July - October.

thegrb93 commented 3 years ago

Might be able to work-around it by waiting another frame.

robotboy655 commented 3 years ago

It just really isn't a gmod bug. We know it works properly with the default duplicator and nothing about constraint entities or Set/Get Pos/Angles has changed in a long time.

You can try addonsystem beta branch (code:goodpassword), which is very outdated and see if happens there.

aerofur commented 3 years ago

Can test in an hour or so.

aerofur commented 3 years ago

Had an engine error crash on singleplayer load using the addonsystem dev branch

aerofur commented 3 years ago

Engine Error:

Server Lua Stack Leak [1]!
0> UNKNOWN (nil)
robotboy655 commented 3 years ago

Rip. You could try running minimal amount of addons.

aerofur commented 3 years ago

Yep that fixed it, the moment I spawn my lua SEnt it crashes with the same error haha

aerofur commented 3 years ago

seems to be something with timers? I added a timer to an autorun lua file and it crashed on spawn

thegrb93 commented 3 years ago

My power was out all day. now its back. let me try.

thegrb93 commented 3 years ago

@robotboy655 So far i've confirmed that this code works in that branch

--timer.simple(0.001,function()
    wheelEnt:setPos(wheelPos + offset + awayoffset)
--timer.simple(0.001,function()
    constraint.elastic(1, baseEnt, wheelEnt, 0, 0, baseEnt:worldToLocal(wheelCenterEdge+offset+awayoffset), wheelEnt:worldToLocal(wheelCenterEdge+offset+awayoffset), constant, damping, rdamping, elasticWidth)
--timer.simple(0.001,function()
    wheelEnt:setPos(wheelPos - offset + awayoffset)
--timer.simple(0.001,function()
    constraint.elastic(1, baseEnt, wheelEnt, 0, 0, baseEnt:worldToLocal(wheelCenterEdge-offset+awayoffset), wheelEnt:worldToLocal(wheelCenterEdge-offset+awayoffset), constant, damping, rdamping, elasticWidth)
--timer.simple(0.001,function()
    wheelEnt:setPos(wheelPos)
--end) end) end) end) end)

Now the timers need uncommented. I think that change is related to this issue.

thegrb93 commented 3 years ago

Nevermind... it works again in latest stable now too.

thegrb93 commented 3 years ago

Ah but duplicator fails to get the correct offsets. I'll test that in the old branch.

thegrb93 commented 3 years ago

Same behavior. So I guess that timer to get the constraint info is creating race conditions. Will need to refactor that.

thegrb93 commented 3 years ago

I'll be testing. I think overriding AddConstraintTable is the best way to get rid of the timer.

thegrb93 commented 3 years ago

Nvm, that's before the SetTable()

thegrb93 commented 3 years ago

@TitusStudiosMediaGroup can you try replacing that file with this? https://github.com/wiremod/advdupe2/blob/d2446060cac353438b025edb09d8cdcccbd0989b/lua/advdupe2/sv_misc.lua

It fixes the issue for me. I'll pull it once I know it works for you.

aerofur commented 3 years ago

Yup sure can, I'm in town at the moment I can get to it a little later today, cheers

aerofur commented 3 years ago

Sorry I got a little caught up yesterday haha. The new sv_misc fixes the issue on my side.

thegrb93 commented 3 years ago

alright nice. thanks for verifying.

aerofur commented 3 years ago

The new code has broken "auto-aligning" axis constraints. Only on constraints made after the bug fix

thegrb93 commented 3 years ago

what tool makes those?

aerofur commented 3 years ago

Just the normal axis tool, its when you left click on the first entity, it auto positions it to the next entity (where you click)