-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Support for building selected litematic #4178
base: 1.19.4
Are you sure you want to change the base?
Conversation
logDirect("Pleas provide a valid index."); | ||
} | ||
} else { | ||
baritone.getBuilderProcess().buildOpenLitematic(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
baritone.getBuilderProcess().buildOpenLitematic(LitematicaHelper.getSelectedIndex());
this would remove the need of a second buildOpenLitematic method in BuilderProcess however it also wouldnt be wrong to remove the manual selection entirely and just keep the one that builds the schematic litematica has selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jeah that's right, i added the second one with the idea that you can easily use it through the api, but of course you can just send the command directly to the execute function, what works better anyways often.
generally adding options i think is good but for intuition maybe only using the selected one makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about building placements by name? It's intuitive and with proper tab completion is't easy to use.
For easier API usage I think the best option would be a method taking the desired SchematicPlacement
(Why are we even using indices in the first place?). Adding buildOpenLitematic()
is good for consistency with buildOpenSchematic()
though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
names wont work well because you can have multiple schematics with the same name open. how would you decide which schematic is meant if you have 2 "myHouse.litematic"
as to why integers are used instead of SchematicPlacements thats because SchematicPlacement is a class from litematica and i didnt want to add a dependency to a optional mod to the BuilderProcess this way the only Class with a direct dependency on litematica should be LitematicaHelper and methods of that class should only be called after the LitematicaHelper.isLitematicaPresent() method returned true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added an branch with only the selected placement thojo0/baritone/only_selected_litematic
i will add a pull request if requested for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
names wont work well because you can have multiple schematics with the same name open
We are talking about placements here and I though litematica enforces unique names on those, like it does for selection and subregion names. But apparently you can have indistinguishable placements. Reporting an error for duplicates would be an option, though it's certainly not as nice anymore.
logDirect("Pleas provide a valid index."); | ||
} | ||
} else { | ||
baritone.getBuilderProcess().buildOpenLitematic(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about building placements by name? It's intuitive and with proper tab completion is't easy to use.
For easier API usage I think the best option would be a method taking the desired SchematicPlacement
(Why are we even using indices in the first place?). Adding buildOpenLitematic()
is good for consistency with buildOpenSchematic()
though.
Adds the buildOpenLitematic method without index to check for the currently selected litematic
I think this is more intuitive to use the
@litematic
command instead of just using the first loaded placement if no argument is passed