support color to filled map item #2504
Conversation
|
|
| @@ -19,15 +20,16 @@ public static boolean describes(ObjectTag item) { | |||
| return false; | |||
| } | |||
| Material mat = ((ItemTag) item).getBukkitMaterial(); | |||
| // Leather armor and potions | |||
| // Leather armor, potions and filled map | |||
| @@ -66,7 +68,7 @@ public ObjectTag getObjectAttribute(Attribute attribute) { | |||
| // @mechanism ItemTag.color | |||
| // @group properties | |||
| // @description | |||
| // Returns the color of the leather armor item or potion item. | |||
| // Returns the color of the leather armor item, potion item or filled map item. | |||
plugin/src/main/java/com/denizenscript/denizen/objects/properties/item/ItemColor.java
Outdated
Show resolved
Hide resolved
|
This was not properly tested on real map items - Testing thoroughly is mandatory prior to PRs. |
| if (mat == Material.FILLED_MAP) { | ||
| MapMeta mapMeta = (MapMeta) item.getItemMeta(); | ||
| if (!mapMeta.hasColor()) { | ||
| return BukkitColorExtensions.fromColor(Color.WHITE).getObjectAttribute(attribute.fulfill(1)); |
There was a problem hiding this comment.
Is returning WHITE here correct? Seems like it should probably be null?
There was a problem hiding this comment.
tbh... i dont know which default color of filled_map 😅, that why i put white color. i'll change it asap, thanks for remind me to using null
| if (!mapMeta.hasColor()) { | ||
| return null; | ||
| } | ||
| return BukkitColorExtensions.fromColor((mapMeta.getColor())).getObjectAttribute(attribute.fulfill(1)); |
There was a problem hiding this comment.
minor format - too many parens here
| if (!mapMeta.hasColor()) { | ||
| return null; | ||
| } | ||
| return BukkitColorExtensions.fromColor(mapMeta.getColor()).getObjectAttribute(attribute.fulfill((1))); |
There was a problem hiding this comment.
... you fixed one and unfixed the other???
| @@ -79,6 +81,13 @@ public ObjectTag getObjectAttribute(Attribute attribute) { | |||
| } | |||
| return BukkitColorExtensions.fromColor(pm.getColor()).getObjectAttribute(attribute.fulfill((1))); | |||
There was a problem hiding this comment.
i thought it was correct because i follow on this line.... but, i realize and didn't get how getObjectAttribute work. should i change this line too?
There was a problem hiding this comment.
this whole file is weird and needs cleaning up, but not really required for your PR to fix pre-existing issues (it'd be really handy if you did tho! - check #2355 if you're interested in helping cleanup the codebase)
No description provided.