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

Release vv2.0.0-milestone.3.2 #451

Closed
wants to merge 33 commits into from
Closed

Conversation

raoulvdberge
Copy link
Contributor

Hi @raoulvdberge!
This PR was created in response to a manual trigger of the release workflow here: https://github.com/refinedmods/refinedstorage2/actions/runs/6750150563.
Merging this PR will publish the release.

Merge main into develop branch
Introduced NetworkNodeContainerBlockEntityImpl, that inherits from the API block entity. It only has
 code for activeness checking.

Modified AbstractInternalNetworkNodeContainerBlockEntity to have redstone mode, placed by and
 configuration card code.

Modified cable and network receiver to inherit from NetworkNodeContainerBlockEntityImpl
since they don't need redstone mode
and especially not configuration card support.
An added benefit of partitioning everything by feature.
feat: network node containers can now determine their outgoing connections
…nd querying by container key

This transforms GraphNetworkComponent into an interface and introduces the implementation
in GraphNetworkComponentImpl.
…e activeness

Previously, we would keep a cached "last active" value and update based on that. When it changed, we
would update the node activeness and propagate the block state change.

However, this setup breaks when the network node disconnects while the chunk is unloading.
How so? When the chunk unloads, it would update this "last active" value to "false", update the
network node activeness, and update the block state.
However, the block state updating would fail as
the chunk is currently unloading.
That means that when the network node disconnected,
activeness goes to "false", but the block state would stay "true".

Until now, this wasn't really an issue as we can safely
assume that the network node would become active again
as the chunk loads.

However, now we are adding wireless networking with the
network transmitter and network receiver. If a chunk
with a network receiver unloads, the network connection
would disconnect. If we rejoin the chunk, the activeness
on the block state would be "true", but there is no
network connection! It would only move the block state
to activeness "false" when a change occurs in the activeness,
as we only update the block state activeness when the network node activeness changes.

This change makes it so that we (still) update
the network node activeness, but, now are also allowed to update the block state later if we rejoin
a chunk to make it consistent.

This can cause a flicker at startup as the network boots up.
At startup, it detects that the block state is still active, however, the network
 is not initialized yet. It would send a block state
 update to become inactive and then another block state
 update to become active again as the network is initialized.

This wasn't an issue previously as the cached "last active"
was "false" at startup. Since activeness was also "false" at startup,
it wouldn't detect a change.
If the node goes to active a bit later, it would update the activeness in the node, but
a block state update wouldn't be necessary as it was active already. This essentially eliminated the
"initialization" block state update.

Overall, this change makes sure that activeness
is always consistent with the block state representation, even at startup, as a consequence.

This fixes the network receiver bug mentioned above, but
will also solve general possible block state inconsistency.
The network transmitter now has an indicator
 on the block state whether the connection with the receiver is active.
With not enough energy, it's inactive.
Without network card or without connection, it's in errored state.
Otherwise, it's in active state.

The network transmitter will actively retry to reconnect
if the connection is lost (for example due to chunk unloading).
This bug made it so that every time a network device is placed, it would trigger an extra update
after the initial update to connect
to an network.

This extra update was caused by the fact that the device was
going from inactive to active after the connection was made.

We now only update the network if the direction has changed, since that is the
only component that can influence graph scanning for now.

This bug also caused some flickering with the network transmitter
in combination with redstone mode.
@raoulvdberge raoulvdberge deleted the release/v2.0.0-milestone.3.2 branch November 3, 2023 21:03
Copy link

sonarcloud bot commented Nov 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
2.9% 2.9% Duplication

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant