Suche saubere Alternative / Refactoring-Hilfe

[Saheeda]

Hi,

vorweg: Ich entschuldige mich für den miesen Titel, aber mir fiel kein besserer ein.

Meine Klassen sind Teil eines Brettspiels, ähnlich "Risiko".

Jeder Spieler hat verschiedene Einheiten ("Units"), die Einheiten wiederum können verschieden Eigenschaften haben, welche z.B. den Angriff oder die Lebenspunkte beeinflussen.

Eine dieser Eigenschaften heißt "LifeThief". Greift eine Einheit mit dieser Eigenschaft an, wird der Schaden beim Gegner abgezogen und bei den eigenen Lebenspunkten draufgeschlagen.

Bei allen anderen Eigenschaften wird einfach nur berechnet, um wieviel sich durch sie der Schaden verändert und dementsprechend mit dem Gesamtschaden verrechnet.


So viel zur Vorgeschichte.
Mein Problem ist jetzt, dass die beiden geposteten Abschnitte für mich ziemlich dreckig aussehen, zumal in Unit.getAttacked und LifeThief.executeSkill ein Abschnitt fast dasselbe macht.

Hat von euch vielleicht jemand eine Idee, wie man das sauberer umsetzen könnte?

abstract class Unit{

   /**
    * Attacks single unit. Consideres armor and skills of both parties.
    *
    * @param enemy
    */
    public void attack(final Unit enemy) {
        int damage = this.damage.getPower();

        for (Skill skill : this.skills) {
            if (skill instanceof StatusSkill) {
                damage += skill.executeSkill(this, enemy);
            }
        }
        enemy.getAttacked(damage, this);
    }

   /**
    * Lowers enemy's damage by own protection and skills if possible.
    * @param damage
    * @param enemy
    */
    public void getAttacked(final int damage, final Unit enemy) {

        int receivedDamage = damage;
        for (Skill ownSkill : this.getSkills()) {
            if (ownSkill instanceof StatusSkill) {
                receivedDamage += ownSkill.executeSkill(this, enemy);
            }
        }

        receivedDamage -= this.armor.getProtection();
        this.lifepoints -= receivedDamage;
        if (this.lifepoints <= 0) {
            this.isAlive = false;
        }
    }

}

/**
* Interface StatusSkill. Represent skills that change enemy's or own status
* values. Effect may occurs during attacks but can't be called explicitly.
*
*/
interface StatusSkill extends Skill{}

/**
* Class LifeThief.
* Adds given damage to own lifepoints.
*
*/
class LifeThief implements StatusSkill{
    @Override
    public int executeSkill(Unit self, Unit enemy) {
        int ownLifepoints = self.getLifepoints();
        int damage = self.getDamage().getPower();

        for (Skill enemySkill : enemy.getSkills()) {
            if (enemySkill instanceof StatusSkill
                    && !(enemySkill instanceof LifeThief)) {
                damage -= enemySkill.executeSkill(enemy, self);
            }
        }

        damage -= enemy.getArmor().getProtection();

        self.setLifepoints(ownLifepoints + damage);

        return 0;
    }

}

// Nur Beispiel für eine andere Implementierung einer Skill-Klasse
/**
* Class Magic. Ignores enemy's armor on attack.
*
*/
public class Magic implements StatusSkill {

    /**
    * Enemy's armor is considered in {@link Unit}, this method reverts the
    * effect.
    */
    @Override
    public int executeSkill(final Unit self, final Unit enemy) {
        return enemy.getArmor().getProtection();
    }

}

 

[Cromon]

Hallo Saheeda

Dein Setup schreit förmlich nach ECS (das ist ein Link). LifeThief wäre demnach nur eine Eigenschaft, die mit dem CombatSystem interagiert und z.B. eine Schnittstelle OnDamageDealt anbietet, die vom CombatSystem aufgerufen wird. Alle Eigenschaften wären dann Komponenten der Einheiten.

Java ist bei solchen Sachen etwas behindert (im Sinne von rückständig), aber typische Varianten (Pseudocode) sind:

void dealDamage(Unit source, Unit dest, float damage) {
     source.GetComponentsByType<ICombatComponent>().ForEach(c => c.onDamageDealt(damage));
     dest.GetComponentsByType<ICombatComponent>().ForEach(c => c.onDamageReceived(damage));
}

void LifeThief::onDamageDealt(float damage) {
   this.addHealth(damage);
}

Die Berechnung der Lebenspunkte etc wäre dann auch nur eine Komponente in dest, usw.

Viele Grüsse
Cromon

 

[Saheeda]

Hallo Cromon,

danke für die schnelle Antwort.
ECS schaue ich mir gerade an. Ich hatte schonmal so angefangen zu planen, das dann aber wieder verworfen, weil ich dachte, dass das Murks würde. Tjo, shit happens^^

Ich verstehe deinen geposteten Code nicht so richtig, da ich auch mit Lambda-Ausdrücken noch nicht so viel gearbeitet habe.
Bezieht sich in deinem Beispiel "this.addHealth(damage)" auf die LifeThief-Klasse? Wenn ja, warum? Die Punkte müssten der Unit gut geschrieben werden.

 

[Cromon]

Hallo Saheeda

Also du kannst das natürlich auch mit den in Java etwas eingeschränkten Möglichkeiten machen, mein Code war eher C# orientiert, also sowas:

void dealDamage(Unit source, Unit dest, float damage) {
    for(Component component : source.GetComponents()) {
           if(component instanceof ICombatComponent) {
               ((ICombatComponent) component).onDamageDealt(damage);
           }
    }
    //...
}

Und ja, beim LifeThief würde der natürlich dem Unit, dem die Komponente zugeteilt ist den Damage als HP geben.

Grüsse
Cromon

 

[Saheeda]

Hi, ich bins nochmal,

vielleicht hast du ja auch einen Vorschlag, wie ich mein Levelsystem mit ECS umsetzen kann.

Jede Unit gibt es in 3 Stufen, mit jeder neuen Stufen verändern sich die Eigenschaften, d.h. es kommen neue Komponenten hinzu (und eigentlich müssten sich auch einige verändern).
Bisher hatte ich dazu in jeder Unit eine statische Map, in welcher ich definierte, welche Komponenten bei welchem Level hinzu kommen.

Ich _könnte_ jetzt im LevelSystem eine lange if-else-Kaskade bauen, und abfragen, welchen konkreten Typ die übergeben Unit hat und dementsprechend die richtige Map aufrufen. (Und die Map ins LevelSystem übernehmen.)
=> Ich müsste bei jeder neuen Unit min. 2 Stellen im Code anfassen. Doof.

Oder ich lege in der jeweiligen Unit eine Methode an, welche direkt mit der richtigen Map arbeiten kann.
=> Verletzt das Single-Responsibility und gewissermaßen auch das DRY-Prinzip. Doof.

Egal, wie ichs drehe und wende, es ist alles nicht so das Gelbe vom Ei. :-/

public class LevelSystem {

    public void levelUp(Unit dest) {

        LevelComponent currentLevel = (LevelComponent) dest
                .getComponentByType(LevelComponent.class);

            currentLevel.raise();

        if (dest.getClass().equals(Dragon.class)) {
            dest.addComponents(Dragon.levelBoni.get(currentLevel.getLevel()));
        }
    }

}

public class Dragon extends Unit {

    public static Map<Integer, Collection<Component>> levelBoni;
    static {
        levelBoni = new HashMap<>();
        levelBoni.put(1, Arrays.asList());
        levelBoni.put(2, Arrays.asList(new MagicComponent()));
    }

}

public class LevelComponent implements Component {

    private int level = 1;

    public int getLevel() {
        return this.level;
    }

    public void raise() {
        this.level++;
    }

    @Override
    public String toString() {
        return "LevelComponent [level=" + this.level + "]";
    }
}

 

[Schnoogle]

Hallo,

du könntest dir ja mal das Dekorator-Entwurfsmuster anschauen. Vielleicht kannst du dein Problem damit lösen.

Grüße