[CS2113-T13-4] Rolladie #42
Conversation
…rGuide-v1.0 Update user guide
TVageesan
left a comment
TVageesan
left a comment
There was a problem hiding this comment.
Overall, I think your team needs to agree on a common design as there are two conflicting design patterns of Event Driven design & the Command pattern. It is possible to make these two design patterns work together, but as they are currently implemented they overlap in many functionalities and conflict.
On the other hand, I like that you guys have taken the initiatve to try these patterns and if you can get them working well I think they'll definitely help make your app scalable for 2.0. The individual functions are coded well and there is a good coding standard (other than some violations which I've noted).
| */ | ||
|
|
||
| public class Parser { | ||
| private static final Scanner SCANNER = new Scanner(System.in); |
There was a problem hiding this comment.
I would recommend declaring this as a instance variable here (or in UI class) insted of a constant. The idea behind constants is the value is immutable, i.e. a magic number or string.
While it's clear you don't intend to mutate scanner and thus want to treat it as a constant, the object is inherently mutable (as the reference can be null), so a constant declaration is inappropriate with our coding standards. A better approach would be to instantiate this in Parser (or in future, UI)'s constructor.
| public static String readInput() { | ||
| String inputLine = SCANNER.nextLine().toLowerCase(); | ||
| return inputLine; | ||
| } |
There was a problem hiding this comment.
Is reading in input Parser's responsibility as a class? It might be more appropiate for UI to handle both input and output. This way your Parser is not directly coupled to the terminal input.
| public static String lineSeparator = "======================================================================"; | ||
|
|
||
| /** | ||
| * A string containing the ASCII art logo of the game. | ||
| */ | ||
| public static String logo = |
There was a problem hiding this comment.
constants should be in SCREAMING_SNAKE_CASE
| public static void battleExit(Character player, Character enemy) { | ||
| if (!player.isAlive) { | ||
| System.out.println(enemy.getCharacterName() + " has defeated you. Another one bites the dust!"); | ||
| return; | ||
| } else { | ||
| System.out.println("You have vanquished " + enemy.getCharacterName()); | ||
| } | ||
|
|
||
| if (player.getHealthBars()[0] > 90) { | ||
| System.out.println("\"Ha! that was easy!\", you thought to yourself, unaware of greater dangers that lurk ahead..."); | ||
| } else if (player.getHealthBars()[0] > 50) { | ||
| System.out.println("You dust yourself off, ready for whatever challenge comes your way..."); | ||
| } else { | ||
| System.out.println("You stumble away from the battlefield, severely wounded..."); | ||
| } | ||
| } |
There was a problem hiding this comment.
There seems to be some magic numbers here in how you're setting health thresholds and other values in multiple functions, so a first fix for 1.0 would be to put these in constants.
Secondly, you should consider the principle of separation of concern.
If your UI is printing out a different message based on health threshold or different messages based on attack damage etc, there's actually a tiny bit of game logic getting mixed into the UI class. I don't think it's too bad as of now, but you might want to consider more cleanly dividing the responsibility in 2.0 if possible.
One possible approach could be to extend your current Actions to all return a ActionResult object, which will give the correct message to UI to print. This way UI isn't doing any "Thinking" of it's own. There are many other ways to tackle it, so feel free to experiment.
src/main/java/Game/Rolladie.java
Outdated
| private static final Game GAME = new Game(); | ||
| private static final Parser PARSER = new Parser(); | ||
| private static final Storage STORAGE = new Storage(); |
There was a problem hiding this comment.
Similair to Parser, would recommend doing these as instance variables instead
| ui.printMessage("Leaving so soon? I expected more from you!"); | ||
| System.exit(0); |
There was a problem hiding this comment.
Instead of using System.exit() which is a bit "hacky", it'd be better to gracefully exit in your main class instead.
|
|
||
| import java.util.Random; | ||
|
|
||
| public class Battle extends Event { |
There was a problem hiding this comment.
The class Game.Events.Battle and Game.Battle have identical names, making it a bit confusing to reason with. A simple fix would be renaming this BattleEvent.
They also seem to have overlapping and conflicting logic. You might want to consider rethinking your overall design and consolidating shared/common logic in one class.
| * | ||
| * @return a boolean on whether the player has won the battle. | ||
| */ | ||
| public boolean BattleSequence() { |
Create sequence diagram and update DG
Javadocs and Bug Fix
* 'master' of https://github.com/James17042002/tp: Fix #45 Delete outdated test file Add javadocs for added classes and methods Add skeleton for JUnits testing of Storage, Game and Rolladie Add javadocs for Rolladie, Game and Storage class Bug Fix within Game # Conflicts: # src/main/java/Game/Events/Shop/Shop.java
Create BootsDatabase and WeaponDatabase for predefined equipment.
Completed Loot and Shop event classes + Player and Equipment Logic
* 'master' of https://github.com/James17042002/tp: Create BootsDatabase and WeaponDatabase for predefined equipment.
updated Shop UI for better user experience
Add game menu
Update UG
Fix Parser crashing due to empty entered command
Updated `docs/teams` naming to match requirements
Update AboutUs to correctly display members
…into resolve-conflicts # Conflicts: # src/test/java/equipments/EquipmentTest.java # src/test/java/functions/ui/UITest.java
junit test all working (except for shop test)
Remove old DG and UG pdf
Update aboutUs to correctly show PPP
* 'master' of https://github.com/James17042002/tp: Update AboutUs.md to correctly bring you to PPP Remove old DG and UG pdf #111 update code to resolve check style error #111 test are all working now (except for shop test) i have edited the scanner in UI to be non final bcs i need to reset it to read inputs in my test files #111 update ppp #111 update ppp (shorter version ) # Conflicts: # src/test/java/ShopTest.java
Add Storage Test and Documentations
Fix Earlier PR #142
final check on checkstyle error
8 participants
RollaDie is a Dungeon & Dragons (DnD) text-based RPG, optimized to play using Command Line Interface (CLI) and has a simple text-ui display that reminisces games of the 1960s. This program is meant for CS2113 students as a stress reliever and it aims to provide a fun and replayable experience especially during periods where the student is not connected to the internet.