Skip to content

[CS2113-T11-4] EconoCraft Pro#30

Open
ZMinghuiZ wants to merge 443 commits into
nus-cs2113-AY2324S2:masterfrom
AY2324S2-CS2113-T11-4:master
Open

[CS2113-T11-4] EconoCraft Pro#30
ZMinghuiZ wants to merge 443 commits into
nus-cs2113-AY2324S2:masterfrom
AY2324S2-CS2113-T11-4:master

Conversation

@ZMinghuiZ

Copy link
Copy Markdown

EconoCraft Pro is a single player text adventure game which simulates real-world business challenges, including resource management, investment analysis, and financial planning to help students prepare for the future challenges.​

Comment thread docs/DeveloperGuide.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size of your text and colors is great!

Comment thread docs/DeveloperGuide.md
![Work.png](UML%20diagram%2FWork.png)

The `EconoCraftLogic` mechanism:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some of your diagrams, such as in EconoCraftLogic components, the standard model of excluding icons is not followed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, may I ask what does it mean by excluding icons?

Comment thread docs/DeveloperGuide.md

{Describe the design and implementation of the product. Use UML diagrams and short code snippets where applicable.}

Here is the sequence diagram of the `EconoCraftLogic` executing the command:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sequence diagram following could also benefit from UML notes to help viewers understand instead of just bullet points after the diagram.

Comment thread docs/DeveloperGuide.md

# Implementation

## MiniGame - Typing Game

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The different functionalities of the different mini-games is comprehensively but sensibly written about in the DG. Good job.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Will take note of the issues:)

Comment thread docs/DeveloperGuide.md

## Design & implementation
## Design

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format of DG is neat, well-formatted, easy to understand and consistent. Well Done!

Comment thread docs/DeveloperGuide.md
4. Player would be prompted to make a decision based on the event.
5. Based on the decision made and the event type, the player would receive different rewards or punishments.
6. The event would add uncertainty and excitement to the game.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if the target user for the product and/or the value proposition is specified clearly.

Comment thread docs/DeveloperGuide.md
3. The command would then update the player profile according to the mini-game result.

# Implementation

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be enough examples e.g., sample inputs/outputs. i can understand.

Comment thread docs/DeveloperGuide.md
> - The `XYZ` in `StockXYZ` represents the stock label e.g., `StockOne`, `StockTwo`, etc.

Here is the sequence diagram of the `EconoCraftLogic` executing the command:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notation seems to be compliant with the notation covered in the course.
The diagram is suitable for the purpose it is used for and is not too complicated. Well Done!

@sweijie24 sweijie24 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good job!

Comment thread docs/DeveloperGuide.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DG is neatly arranged with concise explanation, helps with its readability!

Comment thread docs/UML diagram/Work.png

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could standardise the software/theme being used for all sequence diagrams

Comment thread docs/DeveloperGuide.md
2. When these commands are generated and executed in `EconoCraftLogic`, the respective mini-game would be played.
3. The command would then update the player profile according to the mini-game result.

# Implementation

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thorough explanation on the implementation process but would it better if there were also example inputs/output usages as well

Comment thread docs/DeveloperGuide.md

{Describe the value proposition: what problem does it solve?}

## User Stories

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very detailed and fits the scope of the project with regards to the user stories. Good work!

@Cryolian Cryolian left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty good and comprehensive DG overall! Would appreciate having some more visual aids around the text heavy parts

Comment thread docs/DeveloperGuide.md
## Design

### Overview of GameFlow
![GameFlow.jpg](UML%20diagram%2FGameFlow.jpg)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the flow chart. I think it may be more readable if the size of the picture was reduced?

Comment thread docs/DeveloperGuide.md
6. Finally, the `WorkCommand` would update the player profile with the reward earned according to the user's performance in the game.

## MiniGame - Tic Tac Toe

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps some diagrams to represent the architectures of the mini games would be nice.

Comment thread docs/DeveloperGuide.md
7. The process of selling stock can be invoked by the `SellStockCommand` class when user input `sellstock`.
8. The system will calculate user's profit gained at the current price of the stock and return the money back to the
user based on the stock's current price.
9. User can keep the stock they purchased inside their asset for as long as they want.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe consider adding a diagram of what a stock represents, as it has felt quite arbitrary throughout the entire guide.

Comment thread docs/DeveloperGuide.md Outdated
![Save.png](UML%20diagram%2FSave.png)

The mechanism:
1. `Saver` will be called automatically after one command is executed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps consider adding a screenshot of an example saved file, so the readers can visualise it better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants