-
Notifications
You must be signed in to change notification settings - Fork 355
Fix Wire runtime and tool handling edge cases #3648
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
Changes from all commits
eb3560e
d8f5fd3
89f6cf6
27e0cbe
b09d657
3abe3b1
79b276f
1dc2311
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ | |
| "git-hooks-pre-commit", | ||
| "gitrid.sh", | ||
| "LICENSE", | ||
| "wiremod.*" | ||
| "wiremod.*", | ||
| "benchmark_*" | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,8 +58,8 @@ return { | |
| classname = "gmod_wire_customprop", | ||
|
|
||
| readReliableEntity = function(callback) | ||
| index = net.ReadUInt(16) | ||
| creationIndex = net.ReadUInt(32) | ||
| local index = net.ReadUInt(16) | ||
| local creationIndex = net.ReadUInt(32) | ||
|
Comment on lines
+61
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice find. Looks like that the old version had a global pollution |
||
| local startTime = CurTime() | ||
|
|
||
| local function check() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,9 +91,6 @@ local function broadcastSignal(group, name, scope, sender, filter_player) | |
| end | ||
| end | ||
|
|
||
| --local function table_IsEmpty(t) return not pairs(t)(t) end | ||
| local function table_IsEmpty(t) return not next(t) end | ||
|
|
||
| local function setGroup(self, group) | ||
| -- set the current group to the new group | ||
| self.data.signalgroup = group | ||
|
|
@@ -241,8 +238,8 @@ __e2setcost(20) | |
|
|
||
| --- sends signal S to chips owned by the given player, multiple calls for different players do not overwrite each other | ||
| e2function void signalSendToPlayer(string name, entity player) | ||
| if not IsValid(player) then return end | ||
| broadcastSignal(self.data.signalgroup, name, 1, self.entity, player) | ||
| if not IsValid(player) or not player:IsPlayer() then return end | ||
| broadcastSignal(self.data.signalgroup, name, 1, self.entity, player:UniqueID()) | ||
| end | ||
|
|
||
| --[[************************************************************************]]-- | ||
|
|
@@ -253,14 +250,16 @@ registerCallback("construct",function(self) | |
| end) | ||
|
|
||
| registerCallback("destruct",function(self) | ||
| local receiverid = self.entity:EntIndex() | ||
|
|
||
| -- loop through all scopes, ... | ||
| for scope,groups in pairs_ac(scopes) do | ||
| -- ... all groups ... | ||
| for group, signals in pairs_ac(groups) do | ||
| -- ... and all signals ... | ||
| for name, contexts in pairs_ac(signals) do | ||
| -- to remove all signals the chip registered for. | ||
| contexts[self] = nil | ||
| contexts[receiverid] = nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it is better to index by entity index than by entity object, as entity references in indexes are not being GC'd when they become null (being removed). Be careful to when doing this kind of changes, make sure to adjust every instance of such tables.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep @Grocel, I checked the table use before changing this. |
||
| end | ||
| end | ||
| end | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,7 +229,7 @@ FPGAGateActions["execution-previous-normal"] = { | |
| gate.memory = 0 | ||
| end, | ||
| postExecution = function(gate) | ||
| local changed = gate.value != gate.memory | ||
| local changed = gate.value ~= gate.memory | ||
| gate.value = gate.memory | ||
| return changed | ||
| end, | ||
|
|
@@ -254,7 +254,7 @@ FPGAGateActions["execution-previous-vector"] = { | |
| gate.memory = Vector(0, 0, 0) | ||
| end, | ||
| postExecution = function(gate) | ||
| local changed = gate.value != gate.memory | ||
| local changed = gate.value ~= gate.memory | ||
| gate.value = gate.memory | ||
| return changed | ||
| end, | ||
|
|
@@ -279,7 +279,7 @@ FPGAGateActions["execution-previous-angle"] = { | |
| gate.memory = Angle(0, 0, 0) | ||
| end, | ||
| postExecution = function(gate) | ||
| local changed = gate.value != gate.memory | ||
| local changed = gate.value ~= gate.memory | ||
| gate.value = gate.memory | ||
| return changed | ||
| end, | ||
|
|
@@ -304,8 +304,8 @@ FPGAGateActions["execution-previous-string"] = { | |
| gate.memory = "" | ||
| end, | ||
| postExecution = function(gate) | ||
| local changed = gate.value != gate.memory | ||
| local changed = gate.value ~= gate.memory | ||
| gate.value = gate.memory | ||
| return changed | ||
| end, | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What, was that missing before?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes @Grocel, the file was missing the final newline. The actual runtime fix there is the |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1137,8 +1137,8 @@ register("gmod_wire_value", { | |
| VECTOR2 = function(val, e2TypeID) | ||
| if e2TypeID == TYPE_TABLE and #val >= 2 and isnumber(val[1]) and isnumber(val[2]) then return val[1]..", "..val[2] end | ||
| if e2TypeID == TYPE_STRING then | ||
| local x,y,z = string.match( val, "^ *([^%s,]+) *, *([^%s,]+) *$" ) | ||
| if x and y and z then return x..", "..y..", "..z end | ||
| local x,y = string.match( val, "^ *([^%s,]+) *, *([^%s,]+) *$" ) | ||
| if x and y then return x..", "..y end | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice find. This looks like an oversight from the distant past. |
||
| end | ||
|
|
||
| return nil | ||
|
|
@@ -1201,7 +1201,7 @@ register("gmod_wire_value", { | |
| elseif e2TypeID == TYPE_VECTOR then val = {"VECTOR", castE2TypeToWireValueType["VECTOR"](val, e2TypeID)} | ||
| elseif e2TypeID == TYPE_ANGLE then val = {"ANGLE", castE2TypeToWireValueType["ANGLE"](val, e2TypeID)} | ||
| elseif e2TypeID == TYPE_STRING then val = {"STRING", castE2TypeToWireValueType["STRING"](val, e2TypeID)} | ||
| else return "Incorrect 'value' parameter #"..i.." type! Expected table (Ex. table(\"normal\", 0)). Got: "..type( steamid ) end | ||
| else return "Incorrect 'value' parameter #"..i.." type! Expected table (Ex. table(\"normal\", 0)). Got: "..type( val ) end | ||
| elseif not isnumber(val[1]) then -- Plain table | ||
| if TypeID(val[1]) ~= TYPE_STRING then return "Incorrect 'value' parameter #"..i.."[1] type! Expected string ('NORMAL/VECTOR/VECTOR2/VECTOR4/ANGLE/STRING'). Got: "..type( val ) end | ||
|
|
||
|
|
@@ -1214,15 +1214,15 @@ register("gmod_wire_value", { | |
| end | ||
| val = {wireValueType, CastFunc(val[2], TypeID(val[2]))} | ||
| elseif #val == 2 then -- vector2 | ||
| local tempVal = castE2TypeToWireValueType["VECTOR2"](val[2], typeID(val[2])) | ||
| local tempVal = castE2TypeToWireValueType["VECTOR2"](val, TypeID(val)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this right?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes @Grocel. In that branch
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strange how did this happen, that no one found this before? |
||
| if not tempVal then | ||
| return "Incorrect 'value' parameter #"..i.." value! Expected 'VECTOR2'. Got: "..tostring(val[2]) | ||
| end | ||
|
|
||
| val = {"VECTOR2", tempVal} | ||
| elseif #val==4 then -- vector4 | ||
| local tempVal = castE2TypeToWireValueType["VECTOR4"](val[2], typeID(val[2])) | ||
| if not tempVal then return "Incorrect 'value' parameter #"..i.." value! Expected 'VECTOR2'. Got: "..tostring(val[2]) end | ||
| local tempVal = castE2TypeToWireValueType["VECTOR4"](val, TypeID(val)) | ||
| if not tempVal then return "Incorrect 'value' parameter #"..i.." value! Expected 'VECTOR4'. Got: "..tostring(val[2]) end | ||
|
|
||
| val = {"VECTOR4", tempVal} | ||
| else | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1180,7 +1180,10 @@ function WireLib.CanModel(ply, model, skin) | |
| end | ||
|
|
||
| function WireLib.MakeWireEnt( pl, Data, ... ) | ||
| Data.Class = scripted_ents.Get(Data.Class).ClassName | ||
| local sent = scripted_ents.Get(Data.Class) | ||
| if not sent or not sent.ClassName then return false end | ||
|
|
||
| Data.Class = sent.ClassName | ||
|
Comment on lines
+1183
to
+1186
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. It is a more defensive structure. |
||
| if IsValid(pl) and not pl:CheckLimit(Data.Class:sub(6).."s") then return false end | ||
| if Data.Model and not WireLib.CanModel(pl, Data.Model, Data.Skin) then return false end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,16 +36,16 @@ function TOOL:RightClick(trace) | |
|
|
||
| if (trace.Entity and trace.Entity:IsValid()) then | ||
| if (trace.Entity:GetClass() == "prop_physics") then | ||
| self:GetOwner():ConCommand('wire_cd_disk_model "'..trace.Entity:GetModel()..'"\n') | ||
| self:GetOwner():ConCommand('wire_cd_disk_skin "'..trace.Entity:GetSkin()..'"\n') | ||
| self:GetOwner():ConCommand("wire_cd_disk_model \""..trace.Entity:GetModel().."\"\n") | ||
| self:GetOwner():ConCommand("wire_cd_disk_skin \""..trace.Entity:GetSkin().."\"\n") | ||
|
Comment on lines
+39
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Linter change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly yes @Grocel; the quote change is just cleanup while touching the tool. The meaningful change in this file is the lowercase model cvar below. |
||
| end | ||
| end | ||
|
|
||
| return true | ||
| end | ||
|
|
||
| function TOOL.BuildCPanel(panel) | ||
| WireDermaExts.ModelSelect(panel, "wire_cd_disk_Model", list.Get( "Wire_Laser_Disk_Models" ), 1) | ||
| WireDermaExts.ModelSelect(panel, "wire_cd_disk_model", list.Get( "Wire_Laser_Disk_Models" ), 1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lowercase change. Might not do anything, as it probably lowercased internally already. But it makes it more clean. |
||
| panel:NumSlider("Disk density (inches per block, ipb)","wire_cd_disk_precision",1,16,0) | ||
| panel:NumSlider("Inner radius (disk hole radius)","wire_cd_disk_iradius",1,48,0) | ||
| panel:NumSlider("Disk skin (0..8, standard disks only)","wire_cd_disk_skin",0,8,0) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,13 +21,13 @@ if SERVER then | |
| end | ||
| end | ||
|
|
||
| TOOL.ClientConVar[ "Model" ] = "models/jaanus/wiretool/wiretool_siren.mdl" | ||
| TOOL.ClientConVar[ "model" ] = "models/jaanus/wiretool/wiretool_siren.mdl" | ||
| TOOL.ClientConVar[ "outColor" ] = "0" | ||
| TOOL.ClientConVar[ "range" ] = "2000" | ||
|
|
||
| function TOOL.BuildCPanel(panel) | ||
| WireToolHelpers.MakePresetControl(panel, "wire_colorer") | ||
| WireDermaExts.ModelSelect(panel, "wire_colorer_model", list.Get( "Wire_Laser_Tools_Models" ), 1, true) | ||
| panel:CheckBox("#WireColorerTool_outColor", "wire_colorer_outColor") | ||
| panel:NumSlider("#WireColorerTool_Range", "wire_colorer_Range", 1, 10000, 2) | ||
| panel:NumSlider("#WireColorerTool_Range", "wire_colorer_range", 1, 10000, 2) | ||
|
Comment on lines
+24
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that this PR fixes a lot oversights. Convars should always be lowercase. |
||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this file ever used, e.g. by workshop uploads? Hard to imagine that it worked with the syntax error like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah @Grocel, I was not assuming the upload path, just noticed it is invalid JSON as-is. The comma only preserves the existing two ignore entries (
wiremod.*andbenchmark_*) so any consumer that parses it will not choke.