wiremod / wire

Garry's Mod add-on that allows users to wire up components in order to make more elaborate automatic and user-controlled contraptions.
http://www.wiremod.com
Apache License 2.0
549 stars 333 forks source link

E2 Collision Core(now part of propcore) #3099

Closed DerelictDrone closed 1 month ago

DerelictDrone commented 1 month ago

Adds entity collision tracking for E2s.

Collision type (xcd) Has the contents and naming scheme from the Gmod struct of the same name, but without the PhysObj's
Functions table collision:toTable()
Returns a table with the key names and values being the same as the CollisionData struct(minus the physobjs)
Getters

Vectors

HitPos - collisiondata:pos() - collisiondata:hitpos() - collisiondata:position() OurOldVelocity - collisiondata:ouroldvelocity() - collisiondata:entityoldvelocity() TheirOldVelocity - collisiondata:ouroldvelocity() - collisiondata:hitentityoldvelocity() HitNormal - collisiondata:hitnormal() HitSpeed - collisiondata:hitspeed() OurNewVelocity - collisiondata:ournewvelocity() - collisiondata:entitynewvelocity() TheirNewVelocity - collisiondata:theirnewvelocity() - collisiondata:hitentitynewvelocity() OurOldAngularVelocity - collisiondata:ouroldangularvelocity() - collisiondata:entityoldangularvelocity() TheirOldAngularVelocity - collisiondata:ouroldangularvelocity() - collisiondata:hitentityoldangularvelocity()

Entity

HitEntity - collisiondata:hitentity()

Number

Speed - collisiondata:speed() DeltaTime - collisiondata:deltatime() OurSurfaceProps - collisiondata:oursurfaceprops() - collisiondata:entitysurfaceprops() TheirSurfaceProps - collisiondata:theirsurfaceprops() - collisiondata:hitentitysurfaceprops()

Adds event entityCollision(Entity:entity, HitEntity:entity, CollisionData:collision) Called when a tracked entity collides with another entity(does not trigger on hit world)

Tracking function applies the PhysCollide and CallOnRemove callbacks to an entity if it's valid, cleans up on chip destruct, if tracking is stopped manually, or ent is deleted.

Functions number trackCollision(entity ent)
Returns 1 when tracking is successfully started, returns 0 if already tracking this ent or failed to track. Needs event entityCollision

number trackCollision(entity ent, function cb(eexcd) )
Calls callback on collision, then fires the event entityCollision afterward, same functionality as trackCollision but doesn't need event entityCollision to exist

number isTrackingCollision(entity ent)
Returns 1 if ent is being tracked by this chip, 0 if not or if ent is invalid

void stopTrackingCollision(entity ent)
Stops tracking ent, removes callbacks from ent

Open to suggestions

deltamolfar commented 1 month ago

Nice job, but it should be moved to propcore imo, to lesser the sheer amount of vanilla cores, and due to collision tracking having little sense outside of prop manipulation scope.

I would rename their/our prefixes to something more general. Perhaps hitentity/entity?

Would be also nice to have functions throw strict-only errors on attempting to use invalid entity/track already tracked entity, etc, to make more use out of strict mode, and make debuggin for people easier

DerelictDrone commented 1 month ago

Nice job, but it should be moved to propcore imo, to lesser the sheer amount of vanilla cores, and due to collision tracking having little sense outside of prop manipulation scope.

Moved to propcore

I would rename their/our prefixes to something more general. Perhaps hitentity/entity?

Made aliases for our/their getters as entity/hitentity for them, but keeping the original our/theirs around

Would be also nice to have functions throw strict-only errors on attempting to use invalid entity/track already tracked entity, etc, to make more use out of strict mode, and make debuggin for people easier

The 1/0's for the functions now produce runtime errors instead, or in non-strict return 1/0 for success/fail, and trying to check if we're tracking an invalid ent now produces an error too in strict instead

deltamolfar commented 1 month ago

Seems good enough (Soon sf will have no advantages over E2 >:D)

unknao commented 1 month ago

I think this would be a great addition to expression 2, but i would also like to see a way to manipulate what collision group an entity belongs to like the glua function Entity:SetCollisionGroup would allow, maybe even Entity:SetOwner.

deltamolfar commented 1 month ago

@unknao Collision group functionality exists in e2 for ~half a year already :D. And setowner will bring more harm then good imo

CornerPin commented 1 month ago

It would be great if you could add E2Helper descriptions and convert the collision method names to camel case to be consistent with the rest of the E2 functions.

Here are some issues I've found:

if (first()) { ToTrack = noentity() }

if (clk("track")) { trackCollision(ToTrack) }

event entityCreated(Ent:entity) { if (Ent:type() == "prop_physics" & Ent:owner() == owner()) { ToTrack = Ent

    # A delay is needed to order the collision callbacks so that this
    # chip's callback always executes after the vector has been modified.
    timer("track", 0)
}

}

event entityCollision([ ]:entity, Coll:collision) { print("E2: " + Coll:ouroldvelocity()) }

```golo
@name Chip2
@strict

event entityCreated(Ent:entity) {
    if (Ent:type() == "prop_physics" & Ent:owner() == owner()) {
        trackCollision(Ent)
    }
}

event entityCollision([_ _]:entity, Coll:collision) {
    const Vec = Coll:ouroldvelocity() # Or: Coll:toTable()["OurOldVelocity", vector]
    Vec[1] = 1
    Vec[2] = 2
    Vec[3] = 3
}
hook.Add("PlayerSpawnedProp", "e2test", function(_, _, ent)
    timer.Simple(0, function()
        if not ent:IsValid() then return end

        ent:AddCallback("PhysicsCollide", function(_, data)
            print("Lua", data.OurOldVelocity)
        end)
    end)
end)

If you run these scripts and spawn a prop, you will get this output:

Lua 1.000000 2.000000 3.000000
E2: vec(1,2,3)

You can prevent this by copying vectors using the vector constructor before passing them to E2.

DerelictDrone commented 1 month ago

It would be great if you could add E2Helper descriptions and convert the collision method names to camel case to be consistent with the rest of the E2 functions.

E2Helper descriptions have been written

  • GetHitPos will always throw an error, it seems that you forgot to change this to self.

In this particular case I missed changing 'this' to' collision', 'self' refers to the e2 chip and 'this' refers to the object that the :function() was called from.

  • Vectors returned by collision methods should be copied to prevent players from modifying them in other chips and addons.

Thank you for bringing this to my attention especially though, as I was under the impression initially that the physcollide callback wouldn't pass the same table(or contents) to every callback func

wrefgtzweve commented 1 month ago

Would be nice if this could be its own core for more fine-grained control

deltamolfar commented 1 month ago

@wrefgtzweve He made it in it's own core at first, but then moved to propcore. I guess, again, it would be alright if there would be just a convar to disable it, as moving it to complete separate brand new core will just bloat the sheer amount of vanilla cores even more, while also adding a core that have only few functions, single event, and have little to no application if the propcore is disabled. (Nor possibility that this core will grow, as it already does everything its suppose to)

What I would want to test - will this make a new way to spam network? For example you start tracking 100 ents, and then collide hell out of them? If the network do raise significantly - I think silly limit (like 25 ents tracked at the same time) would be sufficient.

wrefgtzweve commented 1 month ago

the entire point of cores is to have fine grained control over them having a convar for something like this seems redundant when cores already exist.

i dont see how networking could be an issue here, it seems all serversided unless i missed something.

my main concern is that a player adds a lot or a few laggy running collision listeners, this will currently lag the server but not add to cputime/ops.

deltamolfar commented 1 month ago

Regarding core/convar convo: Imo - cores are chunk control. You either want all/most of it, or disable it altogether, and as such, having little cores have little sense to me personally (apart for things like http, which are small by size, but are way too different to be placed in any existing core), and convars are the fine grained control we're talking about. But agree to disagree I guess :).

Regarding possible lag: Someone should stress test it, before merging

Denneisk commented 1 month ago

It would be a lot cleaner to use function callbacks instead of events.

thegrb93 commented 1 month ago

Do you have code example for that?

DerelictDrone commented 1 month ago

It would be a lot cleaner to use function callbacks instead of events.

I could probably make an overload for trackCollision that'll store a passed lambda and call it instead of executing the event for that entity, if that's what you mean

deltamolfar commented 1 month ago

I also thought of that, but that is not in style if E2 to use callback functions as arguments at all. A lot of people will not understand this concept, and thus will not use it. (Most of e2 users are not irl programmers)

I guess an additional function with callback could be done, but not by not firing event, as it will break the current events expected behavior. Also no need to not execute event, as people that would want and know how to use callbacks will understand how not to get rekt by both callback and event.

DerelictDrone commented 1 month ago

May not be the most scientific stress test but here's two recordings of the same save, one with an e2 set up to count ALL collisions and one without.

E2 code used:

@outputs Collisions:number Tracked:number
@strict

Collisions = 0
Tracked = 0

event entityCreated(Ent:entity) {
    if(Ent:isValid()) {
        if(!isTrackingCollision(Ent)) {
            trackCollision(Ent)
            Tracked++
        }
    }
}

event entityCollision(Entity:entity, HitEntity:entity, CollisionData:collision) {
    Collisions++
}

https://github.com/user-attachments/assets/7f1cba86-704c-44be-a3a7-258f6962df97

https://github.com/user-attachments/assets/078d707b-0f79-4e3d-b7b6-351cd1fc4fa5

Edit: Here's three saves, 1 has no e2, 2 has 20 that display the number of collisions & tracked entities, and 3 has 20 that display & have a potentially expensive sqrt while loop

deltamolfar commented 1 month ago

Seems fine to me