ProjectKorra Bridge - #425

@xicxen xicxen commented Mar 30, 2024

Copy link

Copy Markdown

Take 3:

pull request #424
pull request #423

lib modifications & extra contributor removed.


 ProjectKorra Bridge initial commit.

1c78ffd

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

// <--[event]

// @Events

// projectkorra player|entity dies|death|killed by <ability>

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

This is a really awkward event format -- maybe do projectkorra <entity> dies with switch by:<ability>

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

// @Triggers when an entity dies from bending.

// @Context

// <context.ability> returns ElementTag(String) of the ability's name.

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

(String) is not a thing, and either way you don't need to specify types so heavily, just <context.ability> returns the ability's name.

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

@Override

public boolean matches(ScriptPath path) {

String ability = path.eventArgLowerAt(4);

// Check if event applies to any ability

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

Don't need comments that just say what the next line is doing (unless there's something unclear that needs explanation)

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

}

@EventHandler

public void onAbilityStart(EntityBendingDeathEvent event) {

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

copy/pasted method name


 Added switches to all the events

504e397

Removed types in @context
Removed comments
Fixed method name

xicxen commented Mar 30, 2024

Copy link

Copy Markdown

Author

Done

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

// <--[event]

// @Events

// projectkorra entity dies|death|killed

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

This should be <entity> ie dynamic match the entity

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

also the three dies/death/killed variants are silly, pick one

mcmonkey4eva reviewed

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java Outdated

@Override

public boolean matches(ScriptPath path) {

if ((path.eventArgLowerAt(3).equals("by")) && (ability == null || !path.tryArgObject(4, ability))) {

@mcmonkey4eva mcmonkey4eva Mar 30, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

this shouldn't be here


 dynamically matches the entity

3784d17

removed variants
removed line 50 (and like from other events)

xicxen commented Mar 31, 2024

Copy link

Copy Markdown

Author

Done

@mcmonkey4eva

mcmonkey4eva commented Mar 31, 2024

Copy link

Copy Markdown

Member

please don't touch the 'resolve' button

xicxen commented Mar 31, 2024

Copy link

Copy Markdown

Author

Understood

tal5

tal5 reviewed

Apr 8, 2024

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java

// -->

public EntityBendingDeathScriptEvent() {

registerCouldMatcher("projectkorra entity killed");

Choose a reason for hiding this comment

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

The meta projectkorra <entity> killed has an entity matcher, but the code doesn't seem to - need to have the same syntax in here, and check for the entity matcher in matches (ScriptPath#tryArgObject)

tal5

tal5 reviewed

Apr 8, 2024

...va/com/denizenscript/depenizen/bukkit/events/projectkorra/EntityBendingDeathScriptEvent.java

@Override

public ScriptEntryData getScriptEntryData() {

return new BukkitScriptEntryData(PlayerTag.mirrorBukkitPlayer(event.getAbility().getPlayer()), null);

@tal5 tal5 Apr 8, 2024

Copy link

Copy Markdown

Member

Choose a reason for hiding this comment

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

Can use the single-entity constructor, I.e. just pass in the player

@mcmonkey4eva

mcmonkey4eva commented Jun 9, 2024

Copy link

Copy Markdown

Member

What's the status here? There's been unaddressed comments waiting for two months now