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

Rework Multiblock Abilities #2420

Open
wants to merge 103 commits into
base: master
Choose a base branch
from

Conversation

ghzdude
Copy link
Contributor

@ghzdude ghzdude commented Mar 19, 2024

What

Reworks IMultiblockAbilityPart to allow for multiple abilities from one part.
MultiblockRecipeLogic checks if an item handler could also be a fluid handler (only for distinct mode atm)
Implements a ghost circuit slot for single-tank fluid input hatches.

Implementation Details

add a class to MultiblockAbility's constructor so that class types can be checked
add a new class to store instances of a MultilbockAbility. types are checked and an exception is thrown when the wrong type is added.
a new class, DualHandler, is added to implement item and fluid handling capabilities. (not used for anything atm)
Should multitank fluid hatches (quad/nonuple) have ghost circuits as well?

Outcome

fluid hatches are more spooky

@ghzdude ghzdude added the type: refactor Suggestion to refactor a section of code label Mar 19, 2024
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 0c22cbd to 6b8c11a Compare March 19, 2024 20:09
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch 2 times, most recently from b58ef1c to f6b840f Compare April 6, 2024 06:48
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 6bdc2d7 to 6d0fcf2 Compare April 14, 2024 16:52
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 6d0fcf2 to dff15e9 Compare July 4, 2024 20:01
@ghzdude ghzdude marked this pull request as ready for review July 4, 2024 23:03
@ghzdude ghzdude requested a review from a team as a code owner July 4, 2024 23:03
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 829ea5f to 04ad422 Compare July 6, 2024 02:51
src/main/java/gregtech/api/DualHandler.java Outdated Show resolved Hide resolved
src/main/java/gregtech/api/DualHandler.java Outdated Show resolved Hide resolved
Comment on lines 352 to 370
for (IMultiblockPart multiblockPart : parts) {
if (multiblockPart instanceof IMultiblockAbilityPart) {
@SuppressWarnings("unchecked")
IMultiblockAbilityPart<Object> abilityPart = (IMultiblockAbilityPart<Object>) multiblockPart;
List<Object> abilityInstancesList = abilities.computeIfAbsent(abilityPart.getAbility(),
k -> new ArrayList<>());
abilityPart.registerAbilities(abilityInstancesList);
for (IMultiblockPart part : parts) {
if (part instanceof IMultiblockAbilityPart abilityPart) {
List<MultiblockAbility> abilityList = abilityPart.getAbilities();
for (MultiblockAbility ability : abilityList) {
List abilityInstancesList = abilities.computeIfAbsent(ability,
k -> new ArrayList<>());
abilityInstancesList.addAll(abilityPart.registerAbilities(ability));
}
Copy link
Member

Choose a reason for hiding this comment

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

What happens here if a single multiblock part has IMPORT_ITEMS and IMPORT_FLUIDS, but the multiblock only allows one of these in the structure at that position. Should the part be allowed or rejected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a method that skips adding abilities at certain positions, but should i just invalidate the multiblock instead?

@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 3700d34 to bfd2e2f Compare July 11, 2024 23:26
@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from fd8136b to 4ac4934 Compare July 27, 2024 00:00
return isExportHatch ? MultiblockAbility.EXPORT_FLUIDS : MultiblockAbility.IMPORT_FLUIDS;
public @NotNull List<MultiblockAbility<?>> getAbilities() {
return isExportHatch ?
Collections.singletonList(MultiblockAbility.EXPORT_FLUIDS) :
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the slots for filling fluid containers technically not count for abilities here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think it makes sense to include the IO slots when searching for a recipe

@@ -86,6 +119,8 @@ public NBTTagCompound writeToNBT(NBTTagCompound data) {
if (locked && lockedFluid != null) {
data.setTag("LockedFluid", lockedFluid.writeToNBT(new NBTTagCompound()));
}
} else {
this.circuitInventory.write(data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is just writing data here correct? Also, why not just serialize the integrated circuit number?

Also, the circuit number needs to be added to initial sync as well, since setting the circuit, exiting the world, and then rejoining will result in the circuit being lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method does just serialize the circuit value, unless you want me to move that logic here and remove the method

import java.util.ArrayList;
import java.util.List;

public class DualHandler implements IItemHandlerModifiable, IMultipleTankHandler, INotifiableHandler {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, missing some annotations in this class, notnull, etc

@Override
public @NotNull IFluidTank getDelegate() {
return this.tank;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these two backwards? Returning the wrong thing for the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with respect to MultiFluidTankEntry, these methods return the correct objects, but their names are reversed

@ALongStringOfNumbers
Copy link
Contributor

Should the reservoir hatch also get a circuit slot?

@ghzdude ghzdude force-pushed the gh-mutli-abilities branch from 7f5f0be to 7e2e27a Compare February 3, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: refactor Suggestion to refactor a section of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants