Skip to content
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

Fix wirelink issues #2942

Merged
merged 3 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion lua/entities/gmod_wire_egp/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ function ENT:Initialize()

self.RenderTable = {}

WireLib.CreateOutputs( self, { "User (Outputs the player who used the screen for a single tick) [ENTITY]", "wirelink [WIRELINK]" } )
WireLib.CreateOutputs(self, { "User (Outputs the player who used the screen for a single tick) [ENTITY]", "wirelink [WIRELINK]" })

WireLib.TriggerOutput(self, "wirelink", self)

self.xScale = { 0, 512 }
self.yScale = { 0, 512 }
Expand Down
9 changes: 6 additions & 3 deletions lua/entities/gmod_wire_egp_hud/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ function ENT:Initialize()
self:SetUseType(SIMPLE_USE)
self:AddEFlags( EFL_FORCE_CHECK_TRANSMIT )

self.Inputs = WireLib.CreateInputs( self, {
"0 to 512 (If enabled, changes the resolution of the egp hud to be between 0 and 512 instead of the user's monitor's resolution.\nWill cause objects to look stretched out on most screens, so your UI will need to be designed with this in mind.\nIt's recommended to use the egpScrW, egpScrH, and egpScrSize functions instead.)",
"wirelink [WIRELINK]"
WireLib.CreateInputs(self, {
"0 to 512 (If enabled, changes the resolution of the egp hud to be between 0 and 512 instead of the user's monitor's resolution.\nWill cause objects to look stretched out on most screens, so your UI will need to be designed with this in mind.\nIt's recommended to use the egpScrW, egpScrH, and egpScrSize functions instead.)"
})

WireLib.CreateOutputs(self, { "wirelink [WIRELINK]" })

WireLib.TriggerOutput(self, "wirelink", self)

self.xScale = { 0, 512 }
self.yScale = { 0, 512 }
self.Scaling = false
Expand Down
3 changes: 2 additions & 1 deletion lua/entities/gmod_wire_expression2/core/custom/cl_wiring.lua
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
E2Helper.Descriptions["createWire(e:essnvs)"] = "Creates a wire from specified input of first entity to the output of second entity, with specified wire width, color and material"
E2Helper.Descriptions["createWire(e:ess)"] = "Creates a wire from specified input of first entity to the output of second entity"
E2Helper.Descriptions["createWirelink(e:)"] = "Creates a wirelink output on the entity"
E2Helper.Descriptions["deleteWire(e:s)"] = "Unwires the specified input of the entity"
E2Helper.Descriptions["getWireInputs(e:)"] = "Returns array of all inputs of the entity"
E2Helper.Descriptions["getWireOutputs(e:)"] = "Returns array of all outputs of the entity"
E2Helper.Descriptions["wirelink(e:)"] = "Returns entity's wirelink"
E2Helper.Descriptions["wirelink(e:)"] = "Returns entity's wirelink"
12 changes: 11 additions & 1 deletion lua/entities/gmod_wire_expression2/core/custom/wiring.lua
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,17 @@ __e2setcost(5)
e2function wirelink entity:wirelink()
if not IsValid(this) then return self:throw("Invalid entity!", nil) end
if not isOwner(self, this) then return self:throw("You do not own this entity!", nil) end


return this
end

e2function wirelink entity:createWirelink()
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of this function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mimics the old entity:wirelink() that would create a wirelink output. You can do the same with the Wire tool, but I guess some people wanted to generate wirelinks using E2 instead.

Copy link
Contributor

@thegrb93 thegrb93 Dec 13, 2023

Choose a reason for hiding this comment

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

Why not add back the old behavior to entity:wirelink instead of adding this new function?

Copy link
Member Author

@Denneisk Denneisk Dec 13, 2023

Choose a reason for hiding this comment

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

I don't see it as necessary. You don't create entity outputs when doing entity. I don't believe E2 users are using the wirelink output since they just needed to use entity:wirelink() to even get the wirelink, so there is no point in using the wirelink output on the entity.

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't fix #2940. You need to revert the change. We don't break backwards compatibility for literally no reason. This is just saving a one liner. My idea was to deprecate this, then add e:getWirelink() for the new behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

The wirelink being created isn't even the problem, it's the valid check on wirelinks expecting an extended field on the entity to be set, because wirelinks have to be extended for no reason.

Copy link
Contributor

@Fasteroid Fasteroid Dec 13, 2023

Choose a reason for hiding this comment

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

Agreed, this doesn't fix #2940, please revert the behavior of e:wirelink()

Copy link
Member Author

Choose a reason for hiding this comment

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

I literally tested it, it's fixed in this PR.

Copy link
Contributor

@Fasteroid Fasteroid Dec 13, 2023

Choose a reason for hiding this comment

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

Nevermind, actually tested and it really is fixed... somehow.

if not IsValid(this) then return self:throw("Invalid entity!", nil) end
if not isOwner(self, this) then return self:throw("You do not own this entity!", nil) end

if not this.extended then
WireLib.CreateWirelinkOutput(self.player, this, { true })
end
return this
end

Expand Down
5 changes: 3 additions & 2 deletions lua/entities/gmod_wire_expression2/core/wirelink.lua
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@
self.triggercache = {}
end)

local FLAG_WL = { wirelink = true }

registerCallback("postexecute", function(self)
for _,ent,portname,value in pairs_map(self.triggercache, unpack) do
WireLib.TriggerInput(ent, portname, value)
WireLib.TriggerInput(ent, portname, value, FLAG_WL)
end

self.triggercache = {}
Expand All @@ -28,7 +30,6 @@

local function validWirelink(self, ent)
if not IsValid(ent) then return false end
if not ent.extended then return false end
if not isOwner(self, ent) then return false end
return true
end
Copy link
Member Author

@Denneisk Denneisk Dec 13, 2023

Choose a reason for hiding this comment

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

This is the cause of #2940. It's not a backwards compatibility problem, it was an oversight. There's no purpose for entities to store this flag if you're using it as a wirelink; there are no unique restrictions for wirelinks. Wirelinks are just entities.

If you can give me a good reason why someone would use the E2-generated wirelink output versus directly using the returned wirelink object, I'll concede, but you're arguing for bad code style.

Copy link
Contributor

@Grocel Grocel Dec 13, 2023

Choose a reason for hiding this comment

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

To be honest extended isn't even a good name for marking a wirelink. A field wirelinkExist or something would have been better. Changing it however could break BC to addons.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest extended isn't even a good name for marking a wirelink. A field wirelinkExist or something would have been better. Changing it however could break BC to addons.

hasWirelink would be best imo

Expand Down Expand Up @@ -294,7 +295,7 @@
--- Returns an array of all the inputs that <this> has without their types. Returns an empty array if it has none
e2function array wirelink:inputs()
if not validWirelink(self, this) then return {} end
if(!this.Inputs) then return {} end

Check warning on line 298 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of '!' and 'not'

local InputNames = {}
for k,v in pairs_sortvalues(this.Inputs, WireLib.PortComparator) do
Expand All @@ -306,7 +307,7 @@
--- Returns an array of all the outputs that <this> has without their types. Returns an empty array if it has none
e2function array wirelink:outputs()
if not validWirelink(self, this) then return {} end
if(!this.Outputs) then return {} end

Check warning on line 310 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of '!' and 'not'

local OutputNames = {}
for k,v in pairs_sortvalues(this.Outputs, WireLib.PortComparator) do
Expand All @@ -318,7 +319,7 @@
--- Returns the type of input that <Input> is in lowercase. ( "NORMAL" is changed to "number" )
e2function string wirelink:inputType(string Input)
if not validWirelink(self, this) then return "" end
if(!this.Inputs or !this.Inputs[Input]) then return "" end

Check warning on line 322 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of '!' and 'not'

local Type = this.Inputs[Input].Type or ""
if Type == "NORMAL" then Type = "number" end
Expand All @@ -327,7 +328,7 @@

--- Returns the type of output that <Output> is in lowercase. ( "NORMAL" is changed to "number" )
e2function string wirelink:outputType(string Output)
if not validWirelink(self, this) then return "" end

Check warning on line 331 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of 'not' and '!'
if(!this.Outputs or !this.Outputs[Output]) then return "" end

local Type = this.Outputs[Output].Type or ""
Expand All @@ -341,7 +342,7 @@

[deprecated]
e2function number wirelink:writeCell(address, value)
if not validWirelink(self, this) then return 0 end

Check warning on line 345 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of 'not' and '!'

if not this.WriteCell then return 0 end
if this:WriteCell(address, value) then return 1 else return 0 end
Expand All @@ -357,7 +358,7 @@

e2function array wirelink:readArray(start, size)
if size < 0 then return {} end
if !validWirelink(self, this) or !this.ReadCell then return {} end

Check warning on line 361 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of '!' and 'not'

self.prf = self.prf + size

Expand All @@ -371,7 +372,7 @@
end

registerOperator("indexset", "xwlnn", "", function(state, this, address, value)
if not validWirelink(state, this) then return end

Check warning on line 375 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of 'not' and '!'

if this.WriteCell then
this:WriteCell(address, value)
Expand Down Expand Up @@ -437,7 +438,7 @@
local function WriteString(self, entity, string, X, Y, textcolor, bgcolor, Flash)
if not validWirelink(self, entity) or not entity.WriteCell then return end

if !isnumber(textcolor)then textcolor = conv(textcolor) end

Check warning on line 441 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of '!' and 'not'
if !isnumber(bgcolor) then bgcolor = conv(bgcolor) end

textcolor = Clamp(floor(textcolor), 0, 999)
Expand All @@ -456,7 +457,7 @@
X = 0
Y = Y + 1
end
local Address = 2*(Y*30+(X))

Check warning on line 460 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Unnecessary parentheses"

Unnecessary parentheses
X = X + 1
if Address>=1080 or Address<0 then return end
entity:WriteCell(Address, Byte)
Expand Down Expand Up @@ -499,7 +500,7 @@

if !isnumber(textcolor)then textcolor = conv(textcolor) end
if !isnumber(bgcolor) then bgcolor = conv(bgcolor) end

Check warning on line 503 in lua/entities/gmod_wire_expression2/core/wirelink.lua

View workflow job for this annotation

GitHub Actions / lint

"Syntax inconsistency"

Inconsistent use of 'not' and '!'
textcolor = Clamp(floor(textcolor), 0, 999)
bgcolor = Clamp(floor(bgcolor), 0, 999)
Flash = Flash ~= 0 and 1 or 0
Expand Down
Loading