Thread Rating:
  • 1 Vote(s) - 5 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Feedback on Recent Changes to MechComp
#1
This thread is about my recently merged PR https://github.com/goonstation/goonstation/pull/1231
First, an apology:
PR#1231 Had scope creep during development, and was wrongly titled as a "Refactor ..." when it deserved to also have "rework" in the title, and as a github tag. Their omission wasn't intentional, it was just scope creep from the initial purpose of the PR, and me forgetting to update the title and tags.

With that said, I want to outline the reworkings that I did, and why I did them. From there, I'm accepting any feedback and persuasive arguments to revert or modify these changes.

Quote:Devices can now only have one connection to to each other device: I.E. You cannot connect a single button to each input of a selection component, only a single input.
First a clarification: This prevents
Code:
Sensor1 -> Flusher1
Sensor1 -> Flusher1
It does not prevent
Code:
Sensor1 -> Flusher1
Sensor1 -> Flusher2
Sensor1 -> Flusher3
Sensor2 -> Flusher1
Sensor2 -> Flusher2
Sensor2 -> Flusher3

This change was made because there was a pre-existing TO-DO comment in code file stating that overlapping connections should not be allowed.
So I figured I may as well finish the other coder's to-do, and I took the easiest option: When connecting two devices, if a connection already exists, silently destroy the existing connection, and allow the new one to be formed.



Quote:Devices must now be within a range of 14 of each other to be connected. Fixes an exploit of connecting devices far far away.
In hindsight, this bullet point wasn't greatly written; here's the context for the change:
When performing mouse drag-drops, I've never thought to walk mid-drag; In the environment of Wi-fi comps being infinite range, and relays existing at-all, I assumed that MechComp devices had a connection range limit. Thus, when I was first informed, through a private Discord message, that connections could be "walked" infinitely far away, I assumed the behavior to be an exploit. (The user never said it was an exploit, it was just the combined medium of PM, and my never having walked a connection before, that lead to me thinking it was an exploit)
Personally, when I've needed long-range, I've either used wifi, or chained relays. The later method was used in making graviton express-lanes that would turn on LEDs along the lanes when in-use, warning users to get the fuck out of the way.
Since the merge, I've had two players say they don't understand wifi (packets) or that they thought the component itself was broken. If anyone's having trouble with MechComp or other game mechanics I highly encourage them to use MHelp. More specifically, when first learning wifi: I encourage turning forward-all on, and filtering off, to allow for listening and receiving all signals. Additionally: Wifi transmissions don't need do be valid DWAINE packets. you can transmit, and receive anything as a message.

I do think that MechComp connections themselves are somewhat "magic" currently, and that "magic" should have a range limit OR become less-magic. Envision this: what if there was wire(s) representing the connection between each device. Sure, you could absolutely connect a button to a teleporter across the station, but it'd take some layout work. Not even necessarily PNet red-wire, maybe just a simple decal "stretched" from device to device. Until the magicness is reduced, I think a range is appropriate.

From a different but very-much related (And merged) PR: https://github.com/goonstation/goonstation/pull/1405
Quote:This will prevent anchored objects from being able to slide tile-to-tile by click-dragging them.
This felt like a bug, and I still think this change is deserved. However, if you are a fan of compact setups, look forward to System86's PR: https://github.com/goonstation/goonstation/pull/1257
Reply
#2
Mechcomp feeling more "real" like this would be cool.
Reply
#3
I am mostly a fan of these changes, having multi-connections seems like it could easily cause issues and infinite range connections always felt kind of weird.
Not that keen on the dragging change, since I mostly used it for making my setups nicely apart so I could easily connect them and then dragging them together so they would take up less space. Tbh, I don't really see any possible abuse cases in the old dragging behavior, but it's no big deal, I suppose.
I *am* very much looking forward to the compact setup PR, hopefully issues with it can be cleared up soon so we can have it ingame. ^_^
Reply
#4
Quote:Devices must now be within a range of 14 of each other to be connected. Fixes an exploit of connecting devices far far away.

Does this mean no more inescapable listening post teletraps? Fuck yes.
Reply
#5
At first I thought this broke my button panel teleport system.

Button panel -> Set ID -> Teleporter
Button panel -> Activate -> Relay
Relay -> Activate -> Teleporter
Relay -> Activate -> Delay
Delay -> Wait 10 -> Change ID of teleporter back to original ID

But it turns out I don't even know if this change does because I can not get a button panel or a delay to change the ID of a teleporter.
Reply
#6
I actually quite like the idea of having to wire together MechComp components, though I'd imagine coding it so that you'd have to actually use wire and make pathways would be a challenge. Though, I will say, having a visible line between components when you hover over one to view their connections would be *wonderful*.
Reply
#7
(07-16-2020, 06:06 AM)Lord Birb Wrote:
Quote:Devices must now be within a range of 14 of each other to be connected. Fixes an exploit of connecting devices far far away.

Does this mean no more inescapable listening post teletraps? Fuck yes.

No, I don't think this applies to teleport components. Those just connect via having the same ID.
This applies to mechcomp connections you make via dragging with a multitool in hand.
Reply
#8
(07-16-2020, 07:11 AM)Grithok Wrote: At first I thought this broke my button panel teleport system.

Button panel -> Teleporter: Set-ID
Button panel -> Relay: send
Relay -> Teleporter: activate 
Relay -> Delay: delay (set to 10 deciseconds)
Delay -> Change ID of teleporter back to original ID

But it turns out I don't even know if this change does because I can not get a button panel or a delay to change the ID of a teleporter.
I changed your notation slightly. Note that the button panel doesn't care which button you pressed, the signal is fired to every outgoing device regardless. Devices (with the exception of the DispatchComponent) only have a single output channel. However, they can have multiple input channels, much like the Telecomp Set_ID & Activate channels, which I've labeled after colons.

So with the setup as written, pressing any button *COULD* quickly set the ID and cause a teleport. However this all relies on race conditions of how Byond handles the "Spawn queue". That is to say, it could try sending before the ID is set, despite the relay in between. Also pressing the Activate button would set the teleporter ID to Activate.

Your options are many, but what I'd recommend:
Leverage the Wifisplit device, and have each button output a "packet-formatted" message. Any ID button sends "ID=Custom_ID_Here" and activate can be "Act=1"
Then, set Wifisplit1 to trigger on "ID", and wifisplit2 to trigger on "Act"
Make sure the delay component is set to REPLACE (Toggle Signal Changing) any signals with the teleporter's receive-ID.
Quote:Button panel -> Wifisplit1: split
Button panel -> wifisplit2: split
Wifisplit1-> Teleporter: set_id
wifisplit2 -> Teleporter: send
wifisplit2 -> Delay: delay
Delay -> Teleporter: set_id




(07-16-2020, 09:42 AM)zjdtmkhzt Wrote:
(07-16-2020, 06:06 AM)Lord Birb Wrote:
Quote:Devices must now be within a range of 14 of each other to be connected. Fixes an exploit of connecting devices far far away.

Does this mean no more inescapable listening post teletraps? Fuck yes.

No, I don't think this applies to teleport components. Those just connect via having the same ID.
This applies to mechcomp connections you make via dragging with a multitool in hand.

Correct. Teleporters still have infinite range within their Z-level, and cannot teleport outside of their Z level...
Notably, in underwater maps, telecomps can connect the trench and station level. (I did not add or change this functionality, it was already there)
Reply
#9
I'll give that a shot. As for not being able to change the teleport ID via components, I am not certain if it related to a reported bug on github about dropping components on the ground not triggering some sort of range check but throwing or dragging them does.

The setup I had was able to activate teleporters with the current live version, but it failed to change the ID of teleporters even after directly connecting a button to a delay to a teleporter or a button panel to a teleporter. If wifi works, then at least there is currently 1 way.

I'll have to see if my issue was related to the reported dropping bug as well.

I'll give a wifi setup a go later.

EDIT: I went to test out the wifi and the issue persists.

A teleporter ID can be changed via a multitool on the teleporter.
A teleporter ID can not currently be changed via any other mechcomp device that I have tried.
Reply
#10
Investigating

Fixed in : https://github.com/goonstation/goonstation/pull/1502
Reply
#11
So I've never actually made a teleporter network before. I think I'd prefer a "central rerouter" style, but I tried a point-to-point...
https://cdn.discordapp.com/attachments/3...nknown.png
[Image: unknown.png]

Each node had this setup:
Quote:ButtonPanel -> tele: ID
ButtonPanel -> delay1: delay
delay1 -> tele: activate
delay1 -> delay2: delay
delay2 -> tele: ID


A button panel had this:
Quote:This is Button Panel.
Buttons:
Label: Send to Vendors, Value: vend
Label: Send to Cargo, Value: cargo

Nothing special on delay1
delay2 was set to replace incoming signals with the teleporter's original ID (so cargo for cargo, vend for vendors)

Worked decently well. Had to wait a fraction of a second after pressing a button to be sent.
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)