Fix Wire runtime and tool handling edge cases by ShiroKSH · Pull Request #3648 · wiremod/wire

ShiroKSH · 2026-07-08T20:46:41Z

Summary

Fix addon metadata JSON so the addon file parses again.
Harden Wire runtime/network paths around compressed payload length, entity validity, WireEnt class lookup, E2 file/signal queues, FPGA options, and sents registry casts.
Correct tool edge cases where upload targets, tool convars, and CD lock setup could point at stale or invalid state.

Why

These are small runtime correctness fixes found while reviewing obvious failure paths. The changes avoid nil indexing, malformed net writes, stale controller/link state, and invalid client/server option data without changing normal tool behavior.

Validation

Parsed addon.json with ConvertFrom-Json.
Ran git diff --check origin/master..HEAD.
Ran glualint 1.29.0 on changed non-E2 Lua files.
E2 core files need the repository CI preprocessing step before plain Lua linting; direct local glualint does not understand e2function syntax.

thegrb93 · 2026-07-08T22:19:19Z

 				end

 				local data = table.concat(tbl)
+				local compressed = util.Compress(data)


Either move this back into the if-branch or change the net.WriteStream to net.WriteStream(compressed, nil, true) and update the receive to add a decompress.

Thanks @thegrb93, good catch. I moved compression back under the small-payload branch and kept the compressed-size guard before the 12-bit WriteUInt path; fallback still uses the existing raw WriteStream path.

thegrb93 · 2026-07-08T22:36:48Z

I think this conflicts with the other currently open CD PR.

Thanks @thegrb93. I dropped lua/entities/gmod_wire_cd_ray.lua from this PR so it no longer overlaps with #3593.

thegrb93 · 2026-07-09T01:38:48Z

-					net.WriteBool(false)
-					net.WriteUInt(#data, 12)
-					net.WriteData(data)
+					local compressed = util.Compress(data)


This is not right. Are you just trying to add a check for if compressed is nil?
If compressed is nil, you can instead do
net.WriteBool(false)
net.WriteUInt(0, 12)

Thanks @thegrb93, yes, I was only trying to guard the nil-compress case. Updated it to keep the inline compressed packet path: write false, then length/data when compression succeeds, or length 0 when it does not. No small-payload stream fallback now.

local compressed = util.Compress(data) or "" might do the trick as well. But that depends on how it is handled on the other side.

Good point @Grocel. Since the receive side branches on the bool and then reads the advertised length, I kept the nil case explicit as length 0 rather than substituting an empty compressed payload. It stays in the same inline path and avoids changing the stream protocol.

ax255 · 2026-07-09T12:08:21Z

This really does look like an automated AI agent going around popular Gmod repo.

Grocel · 2026-07-09T12:19:35Z

+			local folder = fop .. "/"
+			net.WriteUInt(#folder, 16)
+			net.WriteData(folder, #folder)


Is this needed to have the length data twice? Why did the old version (pre PR) had two length parameter? It is also not needed to set the length in net.WriteData as it automaticity takes it from the input string.

https://wiki.facepunch.com/gmod/net.WriteData

Thanks @Grocel. The WriteUInt is the protocol length prefix for the server-side net.ReadData(net.ReadUInt(16)); the folder bug was that it wrote #fop before sending fop .. "/". You are right that the explicit second arg to net.WriteData is redundant with the current API, though. I kept it explicit while fixing the advertised length.

Sending the advertised length separately is not needed, as WriteData already does it for you. According to the wiki You can still read the advertised using net.ReadUInt(16). Even more this seem to be the only place with a separately send length. Other places do not have these.

Edit: My bad, I got a bit confused. Your changes are fine.

@Grocel I removed the redundant WriteData(..., len) args in 79b276f. I kept the WriteUInt though: WriteData does not put a length field into the message, its length arg only limits how many bytes from the string are written. This message sends multiple variable-length names, so the receiver still needs a per-entry boundary before ReadData.

Grocel · 2026-07-09T12:21:01Z

+		local index = net.ReadUInt(16)
+		local creationIndex = net.ReadUInt(32)


Nice find. Looks like that the old version had a global pollution

Grocel · 2026-07-09T12:25:40Z

 			for name, contexts in pairs_ac(signals) do
 				-- to remove all signals the chip registered for.
-				contexts[self] = nil
+				contexts[receiverid] = nil


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.

Yep @Grocel, I checked the table use before changing this. runOnSignal stores by self.entity:EntIndex(), signal dispatch uses receiver IDs, and destruct was the odd cleanup path still indexing by context object.

Grocel · 2026-07-09T12:28:57Z

 		return changed
 	end,
-}
+}


What, was that missing before?

Yes @Grocel, the file was missing the final newline. The actual runtime fix there is the != to ~= replacement in the postExecution checks above.

Grocel · 2026-07-09T12:31:20Z

-						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


Nice find. This looks like an oversight from the distant past.

Grocel · 2026-07-09T12:40:47Z

+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)


I like that this PR fixes a lot oversights. Convars should always be lowercase.

Grocel · 2026-07-09T12:44:18Z

+	CreateConVar("sbox_wire_igniters_maxlen", 30)
+	CreateConVar("sbox_wire_igniters_allowtrgply",1)

 	function TOOL:GetConVars()
-		return self:GetClientNumber( "trgply" )~=0, self:GetClientNumber("range")
+		return self:GetClientNumber("trgply") ~= 0, self:GetClientNumber("range")
 	end


Linter changes and clean up, I guess.

Yep @Grocel, this part is cleanup only; the behavior-relevant change in the file is the lowercase model cvar name.

Grocel · 2026-07-09T12:46:41Z

-					net.WriteBool(false)
-					net.WriteUInt(#data, 12)
-					net.WriteData(data)
+					local compressed = util.Compress(data)


local compressed = util.Compress(data) or "" might do the trick as well. But that depends on how it is handled on the other side.

Grocel · 2026-07-09T12:48:19Z

 end

-Net.Receivers = registered_handlers
+Net.Receivers = registered_handlers


What is that, where did this came from? Was it missing?

Yes @Grocel, it was missing a trailing newline before. No behavior change; just editor/git normalization from touching the file. I can revert the whitespace if you would rather keep the diff tighter.

Nevermind, it looked like that Net.Receivers = registered_handlers was missing.

No worries, easy to miss in the diff. That one was just the EOF newline.

Grocel · 2026-07-09T12:49:49Z

 		"gitrid.sh",
 		"LICENSE",
-		"wiremod.*"
+		"wiremod.*",


Was this file ever used, e.g. by workshop uploads? Hard to imagine that it worked with the syntax error like that.

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.* and benchmark_*) so any consumer that parses it will not choke.

ShiroKSH · 2026-07-09T13:46:56Z

@ax255 fair concern. I know this PR touches several old corners at once, and I have been trimming anything that overlaps with other work or feels too noisy. The remaining changes are meant to be small concrete runtime/lint fixes; happy to split or drop parts if maintainers prefer.

CheezusChrust · 2026-07-09T13:51:06Z

the automated slop dispensers have reached the wire repo too let's go

ShiroKSH · 2026-07-09T13:56:07Z

the automated slop dispensers have reached the wire repo too let's go

:с im am sloop its sound like "not happy"

ax255 · 2026-07-09T13:57:31Z

the automated slop dispensers have reached the wire repo too let's go

:с im am sloop its sound like "not happy"

finally got a real human response

Grocel · 2026-07-09T14:05:20Z

I would like to give this PR an honest and fair chance.

Edit: It's over, no pass from me.

Grocel

These new changes (79b276f) are fine.

Astralcircle · 2026-07-09T14:28:33Z

 e2function void printCaption(string text, number duration)
 	send_caption(self, text, duration, false)
-end
+end


Don't do unnecessary styling changes until it changes something behavior

Grocel · 2026-07-09T14:50:27Z

@ShiroKSH: What are you even doing?

ShiroKSH · 2026-07-09T14:55:40Z

@ShiroKSH: What are you even doing?

checking public repos with me team and PR, clearly it's can be not good bt i do it and its all

blueshank-gh · 2026-07-09T15:14:49Z

its insane to think that someone would go this low for a PR...
or lets say 15+ repos of PRs, with AI generation.

as it is my personal opinion.
you have a responsibility to understand what you put in a PR and to reply with respect to the maintainers.
you clearly do not have any respect for the people of this public repository and chose to do this likely to 15+ other repositories.
its critical that a human counterpart is present in the PR creation and the implementation, clearly that's not visible judging by the replies you have made.

for the future please in my opinion, do not just blind-side implementation with AI.

blueshank-gh · 2026-07-09T15:23:01Z

I don't have a say in this obviously but I vote to have this PR closed for irresponsible usage of AI.
I am not hating on AI, it has its wins but clearly the choices here have shown that the usage of AI has been wildly put in with no human intervention or decision making.

ShiroKSH · 2026-07-09T15:27:33Z

I don't have a say in this obviously but I vote to have this PR closed for irresponsible usage of AI. I am not hating on AI, it has its wins but clearly the choices here have shown that the usage of AI has been wildly put in with no human intervention or decision making.

I don't know why you think it isn't used with controls, but if that's what you think, buddy, that's your right. In today's world, we have to admit that AI and AI agents have long since become mainstream, and generally speaking, if users believe they're preventing something with AI, well, if you don’t believe that I’m not an AI, just add a ban on AI agents to MD, and they’ll simply refuse when you ask them to do something. Here’s an example:
https://github.com/ggml-org/llama.cpp/blob/master/AGENTS.md

ByakuyaKuchikie2 · 2026-07-09T15:30:42Z

This is not a corporate repo, this is a community maintained project for the people, by the people. AI has absolutely no place in this project, you need to close this PR and try again without AI

blueshank-gh · 2026-07-09T15:34:58Z

I don't have a say in this obviously but I vote to have this PR closed for irresponsible usage of AI. I am not hating on AI, it has its wins but clearly the choices here have shown that the usage of AI has been wildly put in with no human intervention or decision making.

I don't know why you think it isn't used with controls, but if that's what you think, buddy, that's your right. In today's world, we have to admit that AI and AI agents have long since become mainstream, and generally speaking, if users believe they're preventing something with AI, well, if you don’t believe that I’m not an AI, just add a ban on AI agents to MD, and they’ll simply refuse when you ask them to do something. Here’s an example: https://github.com/ggml-org/llama.cpp/blob/master/AGENTS.md

again AI has its wins, there are clear benefits to it, but just for next time usage, make sure to specify it and understand what its doing, a lot of what is in here doesn't make sense.

mainstream usage of it consist of having a competency to back up the claims yourself on the changes.
its been known that just letting LLMs run wild into PR can cause adverse side effects and defects, its not a tool for replacement, but assistance.

its frustrating to have people generate these and expect the reviewers to go through all of it themselves, find odd styling changes that are not required and ultimately have to deal with histories being changed due to them.

I don't think it should be banned, but rather have some form of competency included when PRs are made from here on out, last thing someone wants is to review a ton of changes that don't play a role into the "feature", and just makes things a living hell for them to review back and forth because the AI either made mistakes, odd styling changes that are not required for said feature, or more.

I am asking that in the future to try to have this level of integrity for PRs, last you want as an individual is to get flamed back and forth for this, let this be a lesson, not scrutiny.

ShiroKSH · 2026-07-09T15:38:25Z

I don't have a say in this obviously but I vote to have this PR closed for irresponsible usage of AI. I am not hating on AI, it has its wins but clearly the choices here have shown that the usage of AI has been wildly put in with no human intervention or decision making.

I don't know why you think it isn't used with controls, but if that's what you think, buddy, that's your right. In today's world, we have to admit that AI and AI agents have long since become mainstream, and generally speaking, if users believe they're preventing something with AI, well, if you don’t believe that I’m not an AI, just add a ban on AI agents to MD, and they’ll simply refuse when you ask them to do something. Here’s an example: https://github.com/ggml-org/llama.cpp/blob/master/AGENTS.md

again AI has its wins, there are clear benefits to it, but just for next time usage, make sure to specify it and understand what its doing, a lot of what is in here doesn't make sense.

mainstream usage of it consist of having a competency to back up the claims yourself on the changes. its been known that just letting LLMs run wild into PR can cause adverse side effects and defects, its not a tool for replacement, but assistance.

its frustrating to have people generate these and expect the reviewers to go through all of it themselves, find odd styling changes that are not required and ultimately have to deal with histories being changed due to them.

I don't think it should be banned, but rather have some form of competency included when PRs are made from here on out, last thing someone wants is to review a ton of changes that don't play a role into the "feature", and just makes things a living hell for them to review back and forth because the AI either made mistakes, odd styling changes that are not required for said feature, or more.

I am asking that in the future to try to have this level of integrity for PRs, last you want as an individual is to get flamed back and forth for this, let this be a lesson, not scrutiny.

Okay, thanks. I understand your point of view. I certainly didn't mean any harm; I actually found the error on my own at first, and as for what happened next, I agree I was just following contributors.md. Since some coding style issues didn’t match my own file, they were corrected thanks (AI) because I sent it the bugs I found for review and to make the code look nice.

blueshank-gh · 2026-07-09T15:40:00Z

Okay, thanks. I understand your point of view. Just to be clear, I didn't mean to cause any harm. I originally found the error on my own, and as for what happened next, I agree I was simply following contributors.md. Since some coding style issues didn't match my own file, they were corrected with

np, best of luck to ya, I'm sure you can make another PR.

ByakuyaKuchikie2 · 2026-07-09T15:40:34Z

I don't have a say in this obviously but I vote to have this PR closed for irresponsible usage of AI. I am not hating on AI, it has its wins but clearly the choices here have shown that the usage of AI has been wildly put in with no human intervention or decision making.

I don't know why you think it isn't used with controls, but if that's what you think, buddy, that's your right. In today's world, we have to admit that AI and AI agents have long since become mainstream, and generally speaking, if users believe they're preventing something with AI, well, if you don’t believe that I’m not an AI, just add a ban on AI agents to MD, and they’ll simply refuse when you ask them to do something. Here’s an example: https://github.com/ggml-org/llama.cpp/blob/master/AGENTS.md

again AI has its wins, there are clear benefits to it, but just for next time usage, make sure to specify it and understand what its doing, a lot of what is in here doesn't make sense.
mainstream usage of it consist of having a competency to back up the claims yourself on the changes. its been known that just letting LLMs run wild into PR can cause adverse side effects and defects, its not a tool for replacement, but assistance.
its frustrating to have people generate these and expect the reviewers to go through all of it themselves, find odd styling changes that are not required and ultimately have to deal with histories being changed due to them.
I don't think it should be banned, but rather have some form of competency included when PRs are made from here on out, last thing someone wants is to review a ton of changes that don't play a role into the "feature", and just makes things a living hell for them to review back and forth because the AI either made mistakes, odd styling changes that are not required for said feature, or more.
I am asking that in the future to try to have this level of integrity for PRs, last you want as an individual is to get flamed back and forth for this, let this be a lesson, not scrutiny.

Okay, thanks. I understand your point of view. I certainly didn't mean any harm; I actually found the error on my own at first, and as for what happened next, I agree I was just following contributors.md. Since some coding style issues didn’t match my own file, they were corrected thanks (AI) because I sent it the bugs I found for review and to make the code look nice.

the good ending

ShiroKSH · 2026-07-09T15:50:33Z

closing and re-pr, for myself

ShiroKSH added 3 commits July 8, 2026 23:44

fix: repair addon metadata

eb3560e

fix: harden wire runtime and net handling

d8f5fd3

fix: correct wire tool upload and convar handling

89f6cf6

ShiroKSH marked this pull request as ready for review July 8, 2026 20:49

fix: satisfy lint for E2 signal changes

27e0cbe

thegrb93 reviewed Jul 8, 2026

View reviewed changes

fix: address runtime review feedback

b09d657

thegrb93 reviewed Jul 9, 2026

View reviewed changes

fix: keep wirenet compression inline

3abe3b1

Grocel reviewed Jul 9, 2026

View reviewed changes

fix: drop redundant net WriteData lengths

79b276f

Grocel reviewed Jul 9, 2026

View reviewed changes

Astralcircle reviewed Jul 9, 2026

View reviewed changes

fix: revert debug newline cleanup

1dc2311

ShiroKSH mentioned this pull request Jul 9, 2026

Fix small Wire runtime edge cases #3649

Draft

ShiroKSH closed this Jul 9, 2026

		local index = net.ReadUInt(16)
		local creationIndex = net.ReadUInt(32)

Uh oh!

Conversation

ShiroKSH commented Jul 8, 2026

Summary

Why

Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thegrb93 Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grocel Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ax255 commented Jul 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grocel Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grocel Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Grocel Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShiroKSH commented Jul 9, 2026

Uh oh!

CheezusChrust commented Jul 9, 2026

Uh oh!

ShiroKSH commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

thegrb93 Jul 9, 2026 •

edited

Loading

Grocel Jul 9, 2026 •

edited

Loading

Grocel Jul 9, 2026 •

edited

Loading

Grocel Jul 9, 2026 •

edited

Loading

Grocel Jul 9, 2026 •

edited

Loading

ShiroKSH commented Jul 9, 2026 •

edited

Loading

Grocel commented Jul 9, 2026 •

edited

Loading

Grocel left a comment •

edited

Loading

blueshank-gh commented Jul 9, 2026 •

edited

Loading

ShiroKSH commented Jul 9, 2026 •

edited

Loading

ShiroKSH commented Jul 9, 2026 •

edited

Loading