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

Determination format modernization Part.2 #2689

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

MrMaleficus
Copy link
Contributor

Updating the determination format for the following ScriptEvents :

  • FurnaceStartsSmeltingScriptEvent
  • RedstoneScriptEvent
  • EntityHealsScriptEvent
  • PlayerCraftsItemScriptEvent
  • PlayerItemTakesDamageScriptEvent

@@ -34,6 +34,9 @@ public class RedstoneScriptEvent extends BukkitScriptEvent implements Listener {

public RedstoneScriptEvent() {
registerCouldMatcher("redstone recalculated");
this.<RedstoneScriptEvent, ElementTag>registerDetermination(null, ElementTag.class, (evt, context, current) -> {
evt.event.setNewCurrent(current.asInt());
Copy link
Member

Choose a reason for hiding this comment

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

these rewrites are removing the error checks that the prior impl had

return switch (name) {
case "location" -> location;
case "item" -> item;
case "recipe_id" -> new ElementTag(Utilities.namespacedKeyToString(event.getRecipe().getKey()), true);
Copy link
Member

Choose a reason for hiding this comment

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

this is a functionality change

@MrMaleficus
Copy link
Contributor Author

Updated

@@ -44,6 +45,11 @@ public class FurnaceStartsSmeltingScriptEvent extends BukkitScriptEvent implemen

public FurnaceStartsSmeltingScriptEvent() {
registerCouldMatcher("furnace starts smelting <item>");
this.<FurnaceStartsSmeltingScriptEvent, DurationTag>registerDetermination(null, DurationTag.class, (evt, context, time) -> {
if (time.asElement().isInt()) {
Copy link
Member

Choose a reason for hiding this comment

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

this still has no error handling that was previously available. Go test a bad determination on an old build vs your version and compare the results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants