Skip to content

Fix small Wire runtime edge cases #3649

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.

Sign up for GitHub

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

Open
ShiroKSH wants to merge 7 commits into
base: master
Choose a base branch
from
+23 −15
Open

Fix small Wire runtime edge cases #3649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3ff85cc
fix: keep reviewed wire runtime fixes
ShiroKSH Jul 9, 2026
a2a9973
fix: drop fpga operator change
ShiroKSH Jul 9, 2026
dba9842
chore: split passed review fixes
ShiroKSH Jul 9, 2026
5fe8d32
fix: keep wirenet fallback inline
ShiroKSH Jul 9, 2026
452f574
fix: remove unused signal helper
ShiroKSH Jul 9, 2026
44fe4aa
fix: align reviewed network payload changes
ShiroKSH Jul 9, 2026
8c1dbef
fix: drop redundant compression size check
ShiroKSH Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lua/entities/gmod_wire_expression2/core/cl_files.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,21 @@ net.Receive( "wire_expression2_request_list", function()

net.Start("wire_expression2_file_list")
local files, folders = file.Find( dir .. "*","DATA" )
net.WriteUInt(#files + #folders, 16)
local txtfiles = {}
for _,fop in pairs(files) do
if string.GetExtensionFromFilename( fop ) == "txt" then
net.WriteUInt(#fop, 16)
net.WriteData(fop)
txtfiles[#txtfiles + 1] = fop
end
end
for _,fop in pairs(folders) do
net.WriteUInt(#txtfiles + #folders, 16)
for _,fop in pairs(txtfiles) do
net.WriteUInt(#fop, 16)
net.WriteData(fop .. "/")
net.WriteData(fop)
end
for _,fop in pairs(folders) do
local folder = fop .. "/"
net.WriteUInt(#folder, 16)
net.WriteData(folder)
Comment on lines +105 to +107

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the "other side" deal with the change? Was it somehow compensated? Can the hardcoded slash be removed or moved to the receiver?

@thegrb93 thegrb93 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing forward-slash was getting trimmed before but now it's not.

wire/lua/entities/gmod_wire_expression2/core/files.lua

Line 447 in fd918b3

table.insert(plist.data, net.ReadData(net.ReadUInt(16)))

Should test that this change in behavior is ok.

There's another bug on line 95 I just noticed where non-txt files will cause a mismatch in amount of data written and number of files specified at line 93.

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is tested, I would give it a pass.

@ShiroKSH ShiroKSH Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the file list roundtrip in GMod with file and folder entries. The folder keeps the trailing /, and the count now only includes the transmitted .txt files, so non-txt files do not shift the receiver.

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thegrb93 What do you thing? It seems to be the only point left before passing.

@thegrb93 thegrb93 Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks.

@Grocel Grocel Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that being said I have no concerns left about this PR.

end
net.SendToServer()
end)
11 changes: 5 additions & 6 deletions lua/entities/gmod_wire_expression2/core/signal.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed. But I would avoid adding more stuff during the review process. This change can stay in, though.

local function setGroup(self, group)
-- set the current group to the new group
self.data.signalgroup = group
Expand Down Expand Up @@ -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())

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not passed. As far as I have been told player:UniqueID() is not expected there. This needs testing.

@thegrb93 thegrb93 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ShiroKSH/wire/blob/452f57412fc23530d3b2892d0ad2f4b1744e2a71/lua/entities/gmod_wire_expression2/core/signal.lua#L72

@thegrb93 thegrb93 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright.

end

--[[************************************************************************]]--
Expand All @@ -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

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not passed. I am too unsure about that one too. This needs testing.

@thegrb93 thegrb93 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct judging from https://github.com/ShiroKSH/wire/blob/452f57412fc23530d3b2892d0ad2f4b1744e2a71/lua/entities/gmod_wire_expression2/core/signal.lua#L126

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine, I guess.

end
end
end
Expand Down
12 changes: 8 additions & 4 deletions lua/wire/wirenet.lua
View file
Open in desktop

@thegrb93 thegrb93 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all wrong. Please change it back to how it was in the first pr.

@ShiroKSH ShiroKSH Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets difficult to keep track of, lol.

Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@ if SERVER then
local data = table.concat(tbl)

if #data < 4096 then
data = util.Compress(data)
local compressed = util.Compress(data)
net.WriteBool(false)
net.WriteUInt(#data, 12)
net.WriteData(data)
if compressed then
net.WriteUInt(#compressed, 12)
net.WriteData(compressed)
else
net.WriteUInt(0, 12)
end
Comment on lines +52 to +59

@Grocel Grocel Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the receiver reflect these changes too? I don't see it in this PR.

@thegrb93 thegrb93 Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now

else
net.WriteBool(true)
net.WriteStream(data, nil, false)
Expand Down Expand Up @@ -207,4 +211,4 @@ function Net.Trivial.Receive(name, callback)
update_handlers(name:lower(), callback)
end

Net.Receivers = registered_handlers
Net.Receivers = registered_handlers
Loading