Implementation of Guardian/ElderGuardian API

aramperes commented 5 years ago

Disclaimer for new contributors

Glowstone is an open-source project under the MIT license. In addition, we have a Contributors License Agreement which includes restrictions on the source of contributions (e.g. decompiled code from Mojang, Spigot, etc.). You will be asked to sign the CLA when submitting your first Pull Request.

Task

In a previous API update, the [Guardian]() API was split into the Guardian and [ElderGuardian]() interfaces. The implementations (GlowGuardian, GlowElderGuardian) need to reflect these changes.

Details

In GlowGuardian:
- Make isElder() return false.
- Make setElder(boolean)...:
  - summon a new GlowElderGuardian at the same position and remove the current entity, only if isElder() return false and the parameter is true.
  - summon a new GlowGuardian at the same position and remove the current entity, only if isElder() return true and the parameter is false.
- In GlowElderGuardian:
- Make isElder() return true.

Need help?

Come chat with us on Discord!

ooxaam commented 5 years ago

Hi, I have started working on this issue,, I need some guidance,, What do we mean by remove the current entity? I have done following changes, but not sure how to remove current entity.

Please see below:

GlowGuardian.java

@Override
    public void setElder(boolean elder) {
        //TODO - 1.11 Field has been removed
        //metadata.setBit(MetadataIndex.GUARDIAN_FLAGS, 0x04, elder);
        if(!isElder() && elder) {
            GlowElderGuardian glowElderGuardian = new GlowElderGuardian(this.location);
        }else if (!elder && isElder()) {
            GlowGuardian glowGuardian = new GlowGuardian(this.location);
        }
    }

aramperes commented 5 years ago

@ooxaam Thanks for your interest! You can remove the current entity by using this.remove().

Pr0methean commented 5 years ago

According to this, making "elder" a mutable flag -- which is what I'd normally do -- would break the documented contract of the Guardian interface. https://hub.spigotmc.org/javadocs/bukkit/org/bukkit/entity/Guardian.html

Pr0methean commented 5 years ago

Location isn't the only attribute we may need to copy, so I'm using an NBT round-trip to ensure everything's included.

Pr0methean commented 5 years ago

Reassign to me if nobody fixes it during Hacktoberfest. In the meantime, the guardian branch is my unfinished solution (tests aren't passing because world.getEntity(UUID) is returning null, even with all the thenCallRealMethod() calls I can tell are necessary).

GlowstoneMC / Glowstone