From 80c77b7956b98f64b5b98947e5085b8b8f82d1fe Mon Sep 17 00:00:00 2001 From: punchready Date: Mon, 3 May 2021 19:25:36 +0200 Subject: [PATCH] refactor: move event checks from builder to their respective classes --- .../gamelibrary/events/CharacterEvent.java | 44 ++++ .../gamelibrary/events/CustomEvent.java | 18 ++ .../gamelibrary/events/EntityEvent.java | 37 +++ .../marvelous/gamelibrary/events/Event.java | 8 + .../gamelibrary/events/EventBuilder.java | 218 +----------------- .../gamelibrary/events/GameEvent.java | 53 +++++ .../gamelibrary/events/GameStateEvent.java | 21 ++ .../gamelibrary/events/EventBuilderTest.java | 81 ++++--- 8 files changed, 232 insertions(+), 248 deletions(-) diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CharacterEvent.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CharacterEvent.java index 0efc37c..f1f3ba9 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CharacterEvent.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CharacterEvent.java @@ -47,6 +47,50 @@ public class CharacterEvent extends Event { return this; } + @Override + public boolean check() { + if(!super.check()) { + return false; + } + + switch(type) { + // Melee- and ranged attacks need the same properties (except for the type) + case MeleeAttackEvent: + case RangedAttackEvent: + if (this.originField == null || + this.targetField == null || + this.originEntity == null || + this.targetEntity == null) { + return false; + } + break; + + // Exchanging and using InfinityStones both uses the same keys. Hereby, using a stone like the + // RealityStone causes the stone to be used on oneself + case ExchangeInfinityStoneEvent: + case UseInfinityStoneEvent: + if (this.originField == null || + this.targetField == null || + this.originEntity == null || + this.targetEntity == null || + this.stoneType == null) { + return false; + } + break; + + // MoveEvents take an originField, a targetField, and an originEntity. + case MoveEvent: + if (this.originField == null || + this.targetField == null || + this.originEntity == null) { + return false; + } + break; + } + + return true; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CustomEvent.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CustomEvent.java index ba43685..395f0a9 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CustomEvent.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/CustomEvent.java @@ -8,6 +8,24 @@ public class CustomEvent extends Event { public String teamIdentifier; public HashMap customContent; + @Override + public boolean check() { + if(!super.check()) { + return false; + } + + switch(type) { + // CustomEvent only requires the custom data. + case CustomEvent: + if (this.customContent == null) { + return false; + } + break; + } + + return true; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EntityEvent.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EntityEvent.java index f6e7dcb..3e229b6 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EntityEvent.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EntityEvent.java @@ -28,6 +28,43 @@ public class EntityEvent extends Event { return this; } + @Override + public boolean check() { + if(!super.check()) { + return false; + } + + switch(type) { + // DestroyedEntityEvent takes an ID and a field, and destroys the entity if the field is correct + case DestroyedEntityEvent: + if (this.targetField == null || this.targetEntity == null) { + return false; + } + break; + + // TakenDamage, ConsumedAP / MP and Healed all need a targetField, targetEntity and Amount. + case TakenDamageEvent: + case ConsumedAPEvent: + case ConsumedMPEvent: + case HealedEvent: + if (this.targetField == null || + this.targetEntity == null || + this.amount == null) { + return false; + } + break; + + // SpawnEntity needs an entity, of course. + case SpawnEntityEvent: + if (this.entity == null) { + return false; + } + break; + } + + return true; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/Event.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/Event.java index 871597c..c11c4cd 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/Event.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/Event.java @@ -13,6 +13,14 @@ public abstract class Event { return this; } + /** + * Checks whether the event contains all necessary properties according to its {@link EventType}. + * @return True if the event has all required properties set, otherwise false + */ + public boolean check() { + return type != null; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilder.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilder.java index 65ae474..448cdc2 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilder.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilder.java @@ -6,7 +6,6 @@ import uulm.teamname.marvelous.gamelibrary.entities.EntityID; import uulm.teamname.marvelous.gamelibrary.entities.StoneType; import java.lang.reflect.Field; -import java.util.Arrays; import java.util.HashMap; import java.util.StringJoiner; @@ -166,153 +165,6 @@ public class EventBuilder { return this; } - /** - * A method to check whether the current event is actually built correctly. If that is not the case, it - * throws an {@link IllegalStateException}. - * This occurs if for example a property is null even though it shouldn't be. - * The check is based on the EventType. Using this method is strongly recommended when working with - * entities, as it prevents unnoticed bugs before they might happen! - * - * @throws IllegalStateException if the current event is non-valid - */ - public EventBuilder check() throws IllegalStateException { - if (this.type == null) throw new IllegalStateException("The eventType is null"); - else { - switch (this.type) { - // all of those events do not need any extra values except the EventType - case Ack: - case Nack: - case Req: - case PauseStartEvent: - case PauseStopEvent: - case TurnTimeoutEvent: - case DisconnectEvent: - break; - - // GameState needs very specific properties - case GameStateEvent: - if (this.entities == null || - this.turnOrder == null || - this.activeCharacter == null || - this.winCondition == null) { - throwException(); - } - break; - - // CustomEvent needs only... well, CustomContent. Who would've thought! - case CustomEvent: - if (this.customContent == null) { - throwException(); - } - break; - - // DestroyedEntityEvent takes an ID and a field, and destroys the entity if the field is correct - case DestroyedEntityEvent: - if (this.targetField == null || this.targetEntity == null) { - throwException(); - } - break; - - // TakenDamage, ConsumedAP / MP and Healed all need a targetField, targetEntity and Amount. - case TakenDamageEvent: - case ConsumedAPEvent: - case ConsumedMPEvent: - case HealedEvent: - if (this.targetField == null || - this.targetEntity == null || - this.amount == null) { - throwException(); - } - break; - - // SpawnEntity needs an entity, of course. - case SpawnEntityEvent: - if (this.entity == null) { - throwException(); - } - break; - - - // Melee- and ranged attacks need the same properties (except for the type) - case MeleeAttackEvent: - case RangedAttackEvent: - if (this.originField == null || - this.targetField == null || - this.originEntity == null || - this.targetEntity == null) { - throwException(); - } - break; - - // Exchanging and using InfinityStones both uses the same keys. Hereby, using a stone like the - // RealityStone causes the stone to be used on oneself - case ExchangeInfinityStoneEvent: - case UseInfinityStoneEvent: - if (this.originField == null || - this.targetField == null || - this.originEntity == null || - this.targetEntity == null || - this.stoneType == null) { - throwException(); - } - break; - - // MoveEvents take an originField, a targetField, and an originEntity. - case MoveEvent: - if (this.originField == null || - this.targetField == null || - this.originEntity == null) { - throwException(); - } - break; - - // RoundSetupEvents take a RoundCount and a CharacterOrder - case RoundSetupEvent: - if (this.roundCount == null || this.characterOrder == null) { - throwException(); - } - break; - - // TurnEvents take a TurnCount and a NextCharacter - case TurnEvent: - if (this.turnCount == null || this.nextCharacter == null) { - throwException(); - } - break; - - // A WinEvent needs to know what player has won - case WinEvent: - if (this.playerWon == null) { - throwException(); - } - break; - - // TimeoutEvents give a message back. As the only events to do so, this might be removed later on. - case TimeoutEvent: - if (this.message == null) { - throwException(); - } - break; - - // TimeoutWarnings carry a message and the amount of time left in seconds. The message might disappear. - case TimeoutWarningEvent: - if (this.message == null || this.timeLeft == null) { - throwException(); - } - break; - } - } - return this; - } - - /** - * Utility function for throwing a specific exception. - * @throws IllegalStateException if the builder hasn't received enough properties to construct the event - */ - private void throwException() throws IllegalStateException { - throw new IllegalStateException("Properties malformed for " + this.type + ".\n" + "Builder properties: " + this.notNullToString()); - } - /** * Builds a {@link GameEvent} from the values given to the builder. * * - * @param checked Determines if properties should be checked * @return a {@link GameEvent} based on the builder */ - public GameEvent buildGameEvent(boolean checked) throws IllegalStateException { - if(checked) { - this.check(); - } - return buildGameEvent(); - } - - /** - * Builds a {@link GameEvent} from the values given to the builder without checking. - * @return a {@link GameEvent} based on the builder - */ - private GameEvent buildGameEvent() { + public GameEvent buildGameEvent() throws IllegalStateException { var gameEvent = new GameEvent(); gameEvent.type = this.type; gameEvent.roundCount = this.roundCount; @@ -364,21 +204,9 @@ public class EventBuilder { *
  • amount is a generic amount, for example damage taken
    • * * - * @param checked Determines if properties should be checked * @return an {@link EntityEvent} based on the builder */ - public EntityEvent buildEntityEvent(boolean checked) throws IllegalStateException { - if(checked) { - this.check(); - } - return this.buildEntityEvent(); - } - - /** - * Builds an {@link EntityEvent} from the values given to the builder without checking. - * @return an {@link EntityEvent} based on the builder - */ - private EntityEvent buildEntityEvent() { + public EntityEvent buildEntityEvent() throws IllegalStateException { var entityEvent = new EntityEvent(); entityEvent.type = this.type; entityEvent.targetEntity = this.targetEntity; @@ -398,21 +226,9 @@ public class EventBuilder { *
  • targetField is the target field in the form of an {@link IntVector2}
    • * * - * @param checked Determines if properties should be checked * @return a {@link CharacterEvent} based on the builder */ - public CharacterEvent buildCharacterEvent(boolean checked) throws IllegalStateException { - if(checked) { - this.check(); - } - return buildCharacterEvent(); - } - - /** - * Builds a {@link CharacterEvent} from the values given to the builder without checking. - * @return a {@link CharacterEvent} based on the builder - */ - private CharacterEvent buildCharacterEvent() { + public CharacterEvent buildCharacterEvent() throws IllegalStateException { var characterEvent = new CharacterEvent(); characterEvent.type = this.type; characterEvent.originEntity = this.originEntity; @@ -434,21 +250,9 @@ public class EventBuilder { *
  • describes whether the win condition is in effect
    • * * - * @param checked Determines if properties should be checked * @return a {@link GameStateEvent} based on the builder */ - public GameStateEvent buildGameStateEvent(boolean checked) throws IllegalStateException { - if(checked) { - this.check(); - } - return this.buildGameStateEvent(); - } - - /** - * Builds a {@link GameStateEvent} from the values given to the builder without checking. - * @return a {@link GameStateEvent} based on the builder - */ - private GameStateEvent buildGameStateEvent() { + public GameStateEvent buildGameStateEvent() throws IllegalStateException { var gameStateEvent = new GameStateEvent(); gameStateEvent.type = this.type; gameStateEvent.entities = this.entities; @@ -465,21 +269,9 @@ public class EventBuilder { *
  • customContent is a {@link HashMap}<{@link String}, {@link Object}> resembling the JSON keys in the event
    • * * - * @param checked Determines if properties should be checked * @return a {@link CustomEvent} based on the builder */ - public CustomEvent buildCustomEvent(boolean checked) throws IllegalStateException { - if(checked) { - this.check(); - } - return this.buildCustomEvent(); - } - - /** - * Builds a {@link CustomEvent} from the values given to the builder without checking. - * @return a {@link CustomEvent} based on the builder - */ - private CustomEvent buildCustomEvent() { + public CustomEvent buildCustomEvent() throws IllegalStateException { var customEvent = new CustomEvent(); customEvent.type = this.type; customEvent.teamIdentifier = this.teamIdentifier; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameEvent.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameEvent.java index 43d9079..9b8fd74 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameEvent.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameEvent.java @@ -17,6 +17,59 @@ public class GameEvent extends Event { public String message; public Integer timeLeft; + @Override + public boolean check() { + if(!super.check()) { + return false; + } + + switch(type) { + // all of those events do not need any extra values except the EventType + case PauseStartEvent: + case PauseStopEvent: + case TurnTimeoutEvent: + case DisconnectEvent: + break; + + // RoundSetupEvents take a RoundCount and a CharacterOrder + case RoundSetupEvent: + if (this.roundCount == null || this.characterOrder == null) { + return false; + } + break; + + // TurnEvents take a TurnCount and a NextCharacter + case TurnEvent: + if (this.turnCount == null || this.nextCharacter == null) { + return false; + } + break; + + // A WinEvent needs to know what player has won + case WinEvent: + if (this.playerWon == null) { + return false; + } + break; + + // TimeoutEvents give a message back. As the only events to do so, this might be removed later on. + case TimeoutEvent: + if (this.message == null) { + return false; + } + break; + + // TimeoutWarnings carry a message and the amount of time left in seconds. The message might disappear. + case TimeoutWarningEvent: + if (this.message == null || this.timeLeft == null) { + return false; + } + break; + } + + return true; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameStateEvent.java b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameStateEvent.java index 2c0c656..8fdce54 100644 --- a/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameStateEvent.java +++ b/src/main/java/uulm/teamname/marvelous/gamelibrary/events/GameStateEvent.java @@ -13,6 +13,27 @@ public class GameStateEvent extends Event { public EntityID activeCharacter; public Boolean winCondition; + @Override + public boolean check() { + if(!super.check()) { + return false; + } + + switch(type) { + // GameState needs very specific properties + case GameStateEvent: + if (this.entities == null || + this.turnOrder == null || + this.activeCharacter == null || + this.winCondition == null) { + return false; + } + break; + } + + return true; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/test/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilderTest.java b/src/test/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilderTest.java index 0315d78..093a5ee 100644 --- a/src/test/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilderTest.java +++ b/src/test/java/uulm/teamname/marvelous/gamelibrary/events/EventBuilderTest.java @@ -37,7 +37,7 @@ class EventBuilderTest { new EntityID(EntityType.P2, 0), }; - turn = new EntityID(EntityType.P1, 0); + turn = turns[0]; entities = new Entity[]{ new Character( @@ -79,7 +79,6 @@ class EventBuilderTest { 25); filled = new EventBuilder(EventType.CustomEvent) -// .withType(EventType.CustomEvent) .withTargetEntity(new EntityID(EntityType.P1, 1)) .withTargetField(new IntVector2(11, 13)) .withAmount(15) @@ -126,7 +125,11 @@ class EventBuilderTest { var roundSetupEvent = new EventBuilder(EventType.RoundSetupEvent) .withRoundCount(4) .withCharacterOrder(turns) - .buildGameEvent(true); + .buildGameEvent(); + + assertThat(roundSetupEvent.check()) + .isTrue() + .withFailMessage("RoundSetupEvent failed check"); var roundSetupEventBaseline = new GameEvent(); roundSetupEventBaseline.type = EventType.RoundSetupEvent; @@ -139,13 +142,17 @@ class EventBuilderTest { var turnEvent = new EventBuilder(EventType.TurnEvent) .withNextCharacter(turn) - .withRoundCount(5) - .buildGameEvent(false); + .withTurnCount(5) + .buildGameEvent(); + + assertThat(turnEvent.check()) + .isTrue() + .withFailMessage("TurnEvent failed check"); var turnEventBaseline = new GameEvent(); turnEventBaseline.type = EventType.TurnEvent; turnEventBaseline.nextCharacter = turn; - turnEventBaseline.roundCount = 5; + turnEventBaseline.turnCount = 5; assertThat(turnEvent) .isEqualTo(turnEventBaseline) @@ -163,7 +170,11 @@ class EventBuilderTest { .withPlayerWon(5912) .withMessage("This message is very much not useful at all") .withTimeLeft(-144) - .buildGameEvent(false); + .buildGameEvent(); + + assertThat(gameEvent.check()) + .isTrue() + .withFailMessage("GameEvent failed check"); var baseline = new GameEvent(); baseline.type = EventType.DisconnectEvent; @@ -188,7 +199,11 @@ class EventBuilderTest { .withTurnOrder(turns) .withActiveCharacter(turn) .withWinCondition(true) - .buildGameStateEvent(false); + .buildGameStateEvent(); + + assertThat(gameStateEvent.check()) + .isTrue() + .withFailMessage("GameStateEvent failed check"); var baseline = new GameStateEvent(); baseline.type = EventType.ConsumedAPEvent; @@ -216,46 +231,42 @@ class EventBuilderTest { @Test void buildGameStateEventWithTooManyProperties() { - assertThatNoException() - .isThrownBy(() -> new EventBuilder(EventType.Ack) // too many properties is fine - .withAmount(15) // also properties of different EventTypes, they just get ignored - .withEntities(entities) // properties belonging to the same eventType get incorporated into - .withWinCondition(false) // the final event, so they have to be ignored - .buildGameStateEvent(true)); // by the programmer later on + assertThat(new EventBuilder(EventType.Ack) // too many properties is fine + .withAmount(15) // also properties of different EventTypes, they just get ignored + .withEntities(entities) // properties belonging to the same eventType get incorporated into + .withWinCondition(false) // the final event, so they have to be ignored + .buildGameStateEvent() // by the programmer later on + .check()).isTrue(); } @Test void buildGameStateEvent() { - assertThatNoException() - .isThrownBy(() -> new EventBuilder(EventType.Ack) // needs no properties - .buildGameStateEvent(true)); + assertThat(new EventBuilder(EventType.Ack) // needs no properties + .buildGameStateEvent() + .check()).isTrue(); + assertThat(new EventBuilder(EventType.Nack).buildGameStateEvent().check()).isTrue(); - assertThatNoException() - .isThrownBy(() -> new EventBuilder(EventType.Nack).buildGameStateEvent(true)); + assertThat(new EventBuilder(EventType.Req).buildGameStateEvent().check()).isTrue(); - assertThatNoException() - .isThrownBy(() -> new EventBuilder(EventType.Req).buildGameStateEvent(true)); + assertThat(new EventBuilder(EventType.GameStateEvent) // if properties missing throw exception + .withTurnOrder(turns) + .withActiveCharacter(turn) + .buildGameStateEvent() + .check()).isFalse(); - assertThatExceptionOfType(IllegalStateException.class) - .isThrownBy(() -> new EventBuilder(EventType.GameStateEvent) // if properties missing throw exception - .withTurnOrder(turns) - .withActiveCharacter(turn) - .buildGameStateEvent(true)); - - assertThatNoException() - .isThrownBy(() -> new EventBuilder(EventType.GameStateEvent) // no exception if all properties present - .withEntities(entities) - .withTurnOrder(turns) - .withActiveCharacter(turn) - .withWinCondition(false) - .buildGameStateEvent(true)); + assertThat(new EventBuilder(EventType.GameStateEvent) // no exception if all properties present + .withEntities(entities) + .withTurnOrder(turns) + .withActiveCharacter(turn) + .withWinCondition(false) + .buildGameStateEvent() + .check()).isTrue(); } @Test void buildCustomEvent() { // TODO: check CustomEvent validation for correctness } - }