Residence bridge update by davight · Pull Request #397 · DenizenScript/Depenizen

davight · 2022-11-21T11:58:42Z

New events:

- residence player creates residence
- residence gets deleted (cause: <cause>)
- residence raid ends (residence: <name>)
- residence raid starts (residence: <name>)

New tags:

- ResidenceTag.area > Returns CuboidTag of this Residence.
- ResidenceTag.subzones > Returns ListTag(ResidenceTag) of subzones in this Residence.
- residence[<name>] > Returns the ResidenceTag from the given name.

Updated tags to the new registerTag format.
Updated Residence dependency to 5.1.0.0

mcmonkey4eva · 2022-11-22T00:52:50Z

+    // -->
+
+    public PlayerCreatesResidenceScriptEvent() {
+        registerCouldMatcher("residence player creates residence"); registerCouldMatcher("residence player create residence");


This shouldn't both be on one line like this

mcmonkey4eva · 2022-11-22T00:53:02Z

+    // <--[event]
+    // @Events
+    // residence player creates residence
+    // residence player create residence


remove this second variant

mcmonkey4eva · 2022-11-22T00:53:51Z

    // <--[event]
    // @Events
    // residence player enters <'residence'>
+    // residence player enter <'residence'>


mcmonkey4eva · 2022-11-22T00:54:37Z

+    //
+    // @Switch cause:<cause> to only process the event if the cause equals specified cause
+    //
+    // @Triggers when a player deletes a Residence.


This trigger text doesn't make sense to me when looking at the cause list below

mcmonkey4eva · 2022-11-22T00:55:10Z

+    // residence gets deleted
+    // residence get deleted
+    //
+    // @Switch cause:<cause> to only process the event if the cause equals specified cause


matches not equals, need period at end

mcmonkey4eva · 2022-11-22T07:57:45Z

+
+    @EventHandler
+    public void onResidenceRaidStartsScriptEvent(ResidenceRaidStartEvent event) {
+        if (event.getRes() == null) {


When/why would this occur?

mcmonkey4eva · 2022-11-22T07:57:58Z

+    public ResidenceRaidStartEvent event;
+    public ResidenceTag residence;
+    public ListTag attackers;
+    public ListTag defenders;


these fields shouldn't be retained.

tal5 · 2022-12-02T20:49:33Z

    @Override
    public void init() {
-        ObjectFetcher.registerWithObjectFetcher(ResidenceTag.class);
+        ObjectFetcher.registerWithObjectFetcher(ResidenceTag.class, ResidenceTag.tagProcessor);


Should use generateTagBase here instead of adding one manually

tal5 · 2022-12-02T21:11:20Z

+        if (event.getResidence() == null) {
+            return;
+        }


Can this ever happen?

tal5 · 2022-12-02T21:48:54Z

+
+    // <--[event]
+    // @Events
+    // residence gets deleted


Maybe just residence deleted?

tal5 · 2022-12-02T21:51:33Z

+    // @Events
+    // residence gets deleted
+    //
+    // @Switch cause:<cause> to only process the event if the cause matches specified cause.


if the cause of deletion matches

tal5 · 2022-12-02T22:08:38Z

+    // @Triggers when a Residence gets deleted.
+    //
+    // @Context
+    // <context.cause> Returns the cause of deletion. ( Available causes: PLAYER_DELETE, OTHER, LEASE_EXPIRE )


Should match the format used by other events, something like ...deletion. Can be: X, Y, or Z

tal5 · 2022-12-02T22:12:51Z

+    }
+
+    public ResidenceDeleteEvent event;
+    public String cause;


This field is redundant, should use getCause().name() in matches, and the ElementTag enum constructor in getContext

tal5 · 2022-12-02T22:21:45Z

+    }
+
+    public ResidenceRaidEndEvent event;
+    public ClaimedResidence residence;


This field is redundant, can just use event.getRes() directly

tal5 · 2022-12-02T22:23:22Z

+    // @Triggers when a player(s) starts raiding a Residence.
+    //
+    // @Context
+    // <context.residence> Returns a ResidenceTag of residence that is being attacked.


tal5 · 2022-12-03T16:26:00Z

+        switch (name) {
+            case "attackers":
+                ListTag attackers = new ListTag();
+                for (RaidAttacker player : event.getAttackers().values()) {


This appears to be a map of attacker UUIDs to their RaidAttacker objects, if that's the case then can probably use the UUID PlayerTag constructor to save the extra method calls / have cleaner code

tal5 · 2022-12-03T16:28:57Z

+
+    @Override
+    public boolean matches(ScriptPath path) {
+        if (!runGenericSwitchCheck(path, "residence", event.getRes().getName())) {


Should implement advancedMatches in ResidenceTag and use path.tryObjectSwitch here (and probably update other usages as well)

This is still unresolved

tal5 · 2022-12-03T16:31:03Z

+    // @Triggers when a player creates a Residence.
+    //
+    // @Context
+    // <context.residence> Returns the ResidenceTag of created residence.


returns the created ResidenceTag.

tal5 · 2022-12-03T16:37:36Z

+            case "residence":
+                return new ResidenceTag(event.getRes());


Should inline this

tal5 · 2022-12-03T16:40:10Z

+            default:
+                return super.getContext(name);


Should probably be outside the switch for consistency

tal5 · 2022-12-03T16:40:51Z

+        return tagProcessor.getObjectAttribute(this, attribute);
+    }
+
+    public static void registerTags() {


registerTags was replaced with register

tal5 · 2022-12-03T16:43:44Z

-        // Returns the owner of the residence.
+        // Returns a PlayerTag of owner in this Residence.


I think this was better as it was before? the return type is specified in the @returns meta

tal5 · 2022-12-03T16:55:42Z

+        // @returns ListTag
+        // @plugin Depenizen, Residence
+        // @description
+        // Returns a ListTag(ResidenceTag) of subzones in this Residence.


Same here, the returns should specify what object type does the list include (ListTag(ResidenceTag)), and the description doesn't need to mention the exact type, I.e. something like Returns a list of subzones in this Residence.

tal5 · 2022-12-03T17:43:36Z

+        tagProcessor.registerTag(ElementTag.class, "is_within", (attribute, object) -> {
+            if (attribute.hasParam()) {
+                LocationTag location = attribute.paramAsType(LocationTag.class);


Should use the registerTag method for tags with a param, see this for an example

tal5 · 2022-12-03T18:07:42Z

 import com.denizenscript.denizencore.objects.ObjectTag;
-import com.denizenscript.denizencore.tags.Attribute;

 public class ResidenceLocationProperties implements Property {


This (and ResidencePlayerProperties) are extension properties, properties used for the sole purpose of adding tags / mechs; the modern alternative to that is extension classes - here's an example, but feel free to ask on Discord if you need any help converting them

mcmonkey4eva · 2023-01-07T20:31:13Z

What's the status of this PR?

mcmonkey4eva · 2023-01-09T08:51:33Z

+        // @description
+        // Returns the ResidenceTag of given residence name.
+        // -->
+        TagManager.registerTagHandler(ResidenceTag.class, "residence", attribute -> {


This custom registered base tag conflicts with the generateBaseTag

mcmonkey4eva · 2023-01-09T08:52:25Z

-    public ScriptEntryData getScriptEntryData() {
-        return new BukkitScriptEntryData(event.getPlayer());
-    }
+    public ScriptEntryData getScriptEntryData() { return new BukkitScriptEntryData(event.getPlayer()); }


... why did you do this?

mcmonkey4eva · 2023-01-09T08:56:07Z

        // @plugin Depenizen, Residence
        // @description
-        // Returns whether the specified location is within this Residence.
+        // Returns boolean whether the specified location is within this Residence.


This change makes no sense

mcmonkey4eva · 2023-01-09T08:57:04Z

        // @plugin Depenizen, Residence
        // @description
-        // Returns if the location has a residence.
+        // Returns boolean whether the location has a Residence.


Also should not have the random boolean thrown in the middle

mcmonkey4eva · 2023-01-09T08:57:40Z

-    @Override
-    public ObjectTag getObjectAttribute(Attribute attribute) {
+public class ResidencePlayerProperties {
+    public static void register() {


newline above this line

mcmonkey4eva · 2023-01-09T08:57:52Z

        // @plugin Depenizen, Residence
        // @description
-        // Returns whether the player has a main Residence.
+        // Returns boolean whether the player has a main Residence.


tal5 · 2023-01-09T12:02:49Z

+    @Override
+    public ObjectTag getContext(String name) {
+        switch (name) {
+            case "cause": return new ElementTag(event.getCause().name());


Should use the ElementTag enum constructor

tal5 · 2023-01-09T12:05:34Z

+
+    @Override
+    public boolean matches(ScriptPath path) {
+        if (!runGenericSwitchCheck(path, "residence", event.getRes().getName())) {


Should use tryObjectSwitch, and also update the switch meta accordingly

tal5 · 2023-01-09T12:11:55Z


-public class ResidenceLocationProperties implements Property {
+public class ResidenceLocationProperties {
+    public static void register() {


Should be ResidenceLocationExtensions, also newline above register

tal5 · 2023-01-09T12:13:14Z

-
-    @Override
-    public ObjectTag getObjectAttribute(Attribute attribute) {
+public class ResidencePlayerProperties {


Should be ResidencePlayerExtensions

mcmonkey4eva · 2023-01-11T23:41:32Z

+
+    @Override
+    public boolean matches(ScriptPath path) {
+        if (!path.tryObjectSwitch("residence", new ElementTag(event.getRes().getName()))) {


Why is this not using ResidenceTag?

tal5 · 2023-01-11T23:50:26Z

+        // @description
+        // Returns a residence object constructed from the input value.
+        // -->


Should have a Refer to <@link objecttype ResidenceTag>. to match the format used by other constructor tag meta

davight added 9 commits November 18, 2022 19:50

residence plugin update

d044246

added PlayerCreatesResidence event

aea99af

minor change to some events

b15ca89

added ResidenceGetsDeleted event

c2dbd38

added ResidenceRaidStarts/Ends event

f9bae54

added new tags, update to new format

a7a38ee

updated properties to new format

04ec1e9

residence bridge update

a248085

cleanup, removed trusted_players tag

74a06df

mcmonkey4eva reviewed Nov 22, 2022

View reviewed changes

cleanup

4362283

mcmonkey4eva reviewed Nov 22, 2022

View reviewed changes

Comment thread src/main/java/com/denizenscript/depenizen/bukkit/objects/residence/ResidenceTag.java

fixes

f99bc93

tal5 reviewed Dec 2, 2022

View reviewed changes

Comment thread ...a/com/denizenscript/depenizen/bukkit/events/residence/PlayerCreatesResidenceScriptEvent.java

tal5 reviewed Dec 2, 2022

View reviewed changes

tal5 reviewed Dec 3, 2022

View reviewed changes

more fixes

0ff698c

davight added 2 commits January 8, 2023 15:21

pls work

df7fcc4

minor fixes

c121007

mcmonkey4eva reviewed Jan 9, 2023

View reviewed changes

tal5 reviewed Jan 9, 2023

View reviewed changes

more fixes

c534f38

mcmonkey4eva reviewed Jan 11, 2023

View reviewed changes

Comment thread src/main/java/com/denizenscript/depenizen/bukkit/bridges/ResidenceBridge.java

mcmonkey4eva reviewed Jan 11, 2023

View reviewed changes

tal5 reviewed Jan 11, 2023

View reviewed changes

davight added 2 commits January 14, 2023 19:11

morefixes

dec49d1

lib fix?

081c5a8

mcmonkey4eva merged commit d5ea97c into DenizenScript:master Jan 15, 2023

		// Returns the owner of the residence.
		// Returns a PlayerTag of owner in this Residence.

Uh oh!

Conversation

davight commented Nov 21, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcmonkey4eva commented Jan 7, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!