SquidDev-CC / CCTweaks

Random additions to ComputerCraft (somewhat deprecated, use CC-Tweaked if you're on Minecraft 1.12).
MIT License
12 stars 2 forks source link

Network construction happens on tick updates #30

Closed ElvishJerricco closed 9 years ago

ElvishJerricco commented 9 years ago

Modems don't findPeripherals() until tick updates are called on the tile entities. This leads to a couple of things:

  1. When the network is invalidated, it takes a whole tick for peripherals to get found. This can bug out a program on a computer if it tries to access a peripheral during that tick.
  2. The code is a mess. Lots of things have to happen in really strange places to make this work. For example, I'm going to have to implement peripherals detaching from networks when they're broken (when attached via modem) by writing something into onNeighborChanged, which is not where that ought to be.

We should take a look at fixing this after we merge #29

ElvishJerricco commented 9 years ago

Creating a new branch for this now. I'll open a pull request and we can go ahead and start talking about what specifically we will do there.

ElvishJerricco commented 9 years ago

Er. Derp... Can't create the pull request until we have actual commits to pull. To start things off, we'll have to discuss here.

ElvishJerricco commented 9 years ago

Seems to me we'll have to create an entirely new method of networking. Every node will belong to an overarching NetworkController object. This object will do the job of tracking attached peripherals, and appropriately attaching and detaching them.

Network operations:

SquidDev commented 9 years ago

So we're going to have to rewrite the network code? Sigh. There would probably be much better performance and maybe better memory usage but it does seem like a lot of work gone down the drain. I guess in the long term it will be worth it.

ElvishJerricco commented 9 years ago

The way I see it, when we rewrote the network code initially, we tried very hard to keep to vanilla CC's way of doing the networking. I think it just proved to not be worth it, and we're taking the next step.

SquidDev commented 9 years ago

I know. I guess I should have been more foresighted than I was. I think we should go for it, but I'm not sure if it should be a 0.2 or a 0.2.1 release.

ElvishJerricco commented 9 years ago

I think it'd likely have some API breaking changes.

SquidDev commented 9 years ago

I guess, so a 0.2 release it is. (Though I'm pretty sure SemVer allows breaking changes on the 0.* release).

My one question with this is what happens if you break a network in half/join them together. In the case of breaking we'd have to create a new network object and refresh both of them, in the case of joining it would be nice to have the larger network win so we have to do less node discovery.

ElvishJerricco commented 9 years ago

It's up to you if this is in 0.2 or 0.2.1.

Yea severance and assimilation will be a pain in the neck.

When a node is placed, I think it has a couple of ways it can go

Or

Both scenarios treat assimilation as a primitive operation whose implementation is arbitrary (for the moment).

I think the second option is better. It's a little weird to create a network only to immediately get rid of it. But letting the "assimilation code" and the "new node attached code" be the same code simplifies things greatly.

ElvishJerricco commented 9 years ago

Severance would be trickier. Finding which nodes get severed when a cable is broken will be tough.

SquidDev commented 9 years ago

I think this should be part of 0.2, I can imagine going '0.2 is released, I can relax'. whilst this still needs work. We'll have to consider multiparts and covers as they will complicate things slightly as a node can change network without being destroyed.

ElvishJerricco commented 9 years ago

Yea. Getting this all right will be rather tricky.