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

Rewrite #6

Closed
Techcable opened this issue Apr 7, 2015 · 3 comments
Closed

Rewrite #6

Techcable opened this issue Apr 7, 2015 · 3 comments
Labels
Milestone

Comments

@Techcable
Copy link
Member

Combat Tag's current codebase is a mess.
The CombatTag class manages the entire plugin violating the single responsibility principle.
Getters and Setters are missing, the packages don't follow convention, and the code isn't extensible.
I suggest moving to something like CombatTagReloaded, where CombatTagging has an event, that other plugins can hook into.
Compare the combat-tagging code in CombatTag, which handles plugin compatibility, settings, and permissions.

        if (plugin.npcm.isNPC(damaged)) {
            return;
        } //If the damaged player is an npc do nothing

        if (plugin.ctIncompatible.WarArenaHook(damager) && plugin.ctIncompatible.WarArenaHook(damaged)) {
            if (!damager.hasPermission("combattag.ignore")) {
                if (plugin.settings.blockCreativeTagging() && damager.getGameMode() == GameMode.CREATIVE) {
                    damager.sendMessage(ChatColor.RED + "[CombatTag] You can't tag players while in creative mode!");
                    return;
                }

                if (plugin.settings.isSendMessageWhenTagged() && !plugin.isInCombat(damager.getUniqueId())) {
                    String tagMessage = plugin.settings.getTagMessageDamager();
                    tagMessage = tagMessage.replace("[player]", "" + damaged.getName());
                    damager.sendMessage(ChatColor.RED + "[CombatTag] " + tagMessage);
                }
                plugin.addTagged(damager);

                if (plugin.settings.blockFly() && damager.isFlying()) {
                    damager.sendMessage(ChatColor.RED + "[CombatTag] You can't fly in combat!");
                    damager.setFlying(false);
                }
            }
            if (!damaged.hasPermission("combattag.ignore") && !plugin.settings.onlyDamagerTagged()) {
                if (!plugin.isInCombat(damaged.getUniqueId())) {
                    if (plugin.settings.isSendMessageWhenTagged()) {
                        String tagMessage = plugin.settings.getTagMessageDamaged();
                        tagMessage = tagMessage.replace("[player]", damager.getName());
                        damaged.sendMessage(ChatColor.RED + "[CombatTag] " + tagMessage);
                    }
                }
                plugin.addTagged(damaged);

                if (plugin.settings.blockFly() && damaged.isFlying()) {
                    damaged.sendMessage(ChatColor.RED + "[CombatTag] Disabling fly!");
                    damaged.setFlying(false);
                }
            }
        }
}

To the code in combat tag reloaded, which fires an event that other classes and plugins can handle and modify easily:

    public void onDamage(EntityDamageByEntityEvent event) {
        LivingEntity attacker = Utils.getRootDamager(event.getDamager());
        LivingEntity defender = (LivingEntity) event.getEntity();
        if (CombatTagAPI.isNPC(defender) || CombatTagAPI.isNPC(attacker)) return;
        if (attacker instanceof Player) onAttack(CombatPlayer.getPlayer((Player)attacker), defender);
        if (defender instanceof Player) onDefend(CombatPlayer.getPlayer((Player)defender), attacker);
    }

    public void onAttack(CombatPlayer attacker, LivingEntity defender) {
        if (!(defender instanceof Player) && !Utils.getPlugin().getSettings().isTagMobs()) return;
        if (attacker.isTagged()) return;
        CombatTagEvent event = new CombatTagEvent(attacker, defender, TagCause.ATTACK);
        Utils.fire(event);
        if (event.isCancelled()) return;
        attacker.tag();
    }

    public void onDefend(CombatPlayer defender, LivingEntity attacker) {
        if (!(attacker instanceof Player) && !Utils.getPlugin().getSettings().isTagMobs()) return;
        if (defender.isTagged()) return;
        CombatTagEvent event = new CombatTagEvent(defender, attacker, TagCause.DEFEND);
        Utils.fire(event);
        if (event.isCancelled()) return;
        defender.tag();
    }

Of course, I would like @Outfenneced 's opinion on this, as i am proposing a complete rewrite.

@Techcable Techcable added this to the 7.0.0 milestone Apr 12, 2015
@Techcable
Copy link
Member Author

@Outfenneced
#8 #5 and #7 are on hold for this.
I would really like your input before a major change like this.

@Outfenneced
Copy link
Contributor

Go for it.

@Techcable
Copy link
Member Author

@Outfenneced
Thanks

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

No branches or pull requests

2 participants