Fix wirelink issues #2942
Fix wirelink issues #2942
Conversation
- Add createWirelink - Fixed validWirelink expected ent.extended
|
@Grocel You can check for wirelinks with this just by doing function ENT:TriggerInput(name, value, ext)
local wired = self:IsConnectedInputWire(name) or istable(ext) and ext.wirelink |
Thanks, this is very helpful! Is it guaranteed to be passed with every trigger call that might come via wirelink? I think it could be helpful to pass the name of the wirelink it was coming from as well. Like if it was triggered via "wirelinkA" or "wirelinkB" etc. in case of multiple wirelink ports. |
As far as I know, wirelinks are only usable with E2, so this will always exist for a wirelink input.
Wirelinks are just entities and they don't directly retain any information about Wiremod wires, so that's a lot more involved. I would recommend not creating multiple wirelink outputs since they're all the same. |
That's fine for my part. I will apply your hint into my addon soon. I am not that familiar with the current Workshop release scheduling nowadays, but I think it will be quite a while until the next "stable" releases. So I will have to account for the older live version as well. |
| return this | ||
| end | ||
|
|
||
| e2function wirelink entity:createWirelink() |
There was a problem hiding this comment.
What's the point of this function?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Why not add back the old behavior to entity:wirelink instead of adding this new function?
There was a problem hiding this comment.
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.
| return this | ||
| end | ||
|
|
||
| e2function wirelink entity:createWirelink() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| return this | ||
| end | ||
|
|
||
| e2function wirelink entity:createWirelink() |
There was a problem hiding this comment.
Agreed, this doesn't fix #2940, please revert the behavior of e:wirelink()
There was a problem hiding this comment.
I literally tested it, it's fixed in this PR.
There was a problem hiding this comment.
Nevermind, actually tested and it really is fixed... somehow.
| @@ -28,7 +30,6 @@ end | |||
|
|
|||
| 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 | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
To be honest
extendedisn't even a good name for marking a wirelink. A fieldwirelinkExistor something would have been better. Changing it however could break BC to addons.
hasWirelink would be best imo
|
Is #2941 able to be closed then? |
|
Yes. |
createWirelinkto reintroduce creating wirelink outputsTriggerOutputnow sends a table with fieldwirelink = trueto indicate it's a wirelink triggervalidWirelinkwould consider most wirelinks invalidFixes #2940