Repository navigation
Conversation
rahul05ranjan
left a comment
There was a problem hiding this comment.
Nice cleanup — delegating hi-speed reads/writes to the entity behind input A (instead of an unbounded private table) matches how the dataplug already works, and passing ReadFromMe through the pair is a sensible way to bridge both.
A few things I'd tighten before merge:
1. Stale NULL entity in self.Memory will error. Wire_Unlink sets input.Src = nil without calling TriggerInput when the source entity is removed (if (Removing) then return end in wirelib.lua, right before the TriggerInput call). So self.Memory keeps pointing at a removed entity, and the next hi-speed access hits self.Memory.WriteCell / self.Memory.ReadCell with no IsValid guard — indexing a NULL entity throws "Tried to use a NULL entity!". The dataplug guards this with IsValid(...) and OwnMemory.ReadCell. Either mirror that (self.Memory and self.Memory:IsValid() and self.Memory.WriteCell), or clear self.Memory in ENT:OnInputWireUnlink, which Wire_Unlink calls before nil-ing Src.
2. WriteCell swallows the delegated result. In both gmod_wire_plug.lua and gmod_wire_socket.lua the WriteToMe branch calls self.Memory:WriteCell(Address, Value) and then returns true unconditionally. If the target device rejects the address (returns false), the caller is told the write succeeded. return self.Memory:WriteCell(Address, Value) keeps the failure signal intact for hi-speed writers.
3. No recursion guard. If plug A's input A is wired from socket B's output while B's input A is wired from A's output (or an entity is wired to its own output), ReadCell ping-pongs between the pair forever. The dataplug threads an infloop counter (bail at 50) for exactly this; the new ReadCell/WriteCell here would just stack overflow. Copying that guard would also keep behavior consistent across the four plug entities.
Minor: in the non-array ReadCell(ReadFromMe=true) branch, self.Memory:ReadCell(Address) is called without ReadFromMe, so a memory entity that itself forwards (another plug) reads its far side instead of itself — probably fine for this design, but worth a sentence in the PR body if it's intended.
Currently, the number/array plugs have a half baked highspeed implementation.
For some reason each entity has their own memory tables, and neither WriteCell or ReadCell clamp the address to anything besides flooring it, which allows them to store an unlimited amount of data. Values stored here don't even show up in the array or number outputs.
Instead of removing the highspeed interface from them, I took the opportunity to improve it to allow for a highspeed device to connect to a plug/socket in tandem with number/array values. Arrays are readable with highspeed but not writeable to prevent abuse.
The purpose of this is to overcome a shortcoming with the other type of plug/socket; they can send a highspeed signal, but you can't send anything else without mapping it into the same memory space. This requires extra complexity and entities, especially if the number(s) need to trigger something. The changes here entirely solve this problem.
While I'm here, doesn't it seem silly that we have 4 different entities (plug/socket, dataplug/datasocket) where any 2 do almost the exact same thing as each other (even now)? If these plugs were to be given an extra 'Memory' input in conjunction with some additional changes, they could entirely replace the other plug with a similar mechanism I used in #3654. The same mechanism could be improved to remove the need for separate plug/socket entities, thus reducing the count from 4 to 1 while maintaining full backwards compatibility with old dupes.