[CS2113-T14-1] TheGreatDetective by serenakemono · Pull Request #41 · nus-cs2113-AY2122S1/tp

serenakemono · 2021-09-30T15:48:16Z

TheGreatDetective gives curious and adventurous players a chance to solve murder mysteries.

APZH

Week 11 Tutorial

APZH · 2021-10-27T05:29:22Z

+=======
+The class diagram below shows how the parser interacts with the other classes
+
+![Parser class diagram design](./ParserClassDiagram.png)


Should the an abstract class be defined with "<>" or "{}"?

APZH · 2021-10-27T05:33:00Z

+Here’s a (partial) class diagram of the `Command` component:
+
+
+![(partial class) diagram of Command component](./Command_Class_Diagram.png)


Do the inclusion of icons for the 'Duke', 'Command', 'XYZCommand' class follow the class diagram UML standard as discussed in class?

APZH · 2021-10-27T05:35:09Z

+The plot of the game was adopted from one of the games available in the Mini Program in WeChat called Ju Ben Sha. The original story was in Chinese and was translated to English with the help of Google Translate. 

-## Design & implementation
+## Design


May want to increase the font size of your sequence diagrams as it is abit difficult to read.

APZH · 2021-10-27T05:45:33Z

+- Print messages to terminal depending on the scene.
+- Print corresponding output to terminal according to input command.
+
+[UML diagram for Ui](./UiUML.png)


The link to the UML diagram is broken.

Is there supposed to be an image here? As the image link is broken and would it be better if you just include the image in the DG itself instead of having a link for it?

titustortoiseturtle1999 · 2021-10-28T01:07:33Z

+The Sequence Diagram [below](./next_command_sequence_diagram.png) illustrates within the `Command` component for the `execute(ui,investigation,sceneList)` method call of the `NextCommand` class.
+
+
+![Sequence diagram for execute(ui,investigation,sceneList) method call of NextCommand](./next_command_sequence_diagram.png)


The diagram has many components that are difficult to make out as the font is too small and some of the text is blurry.

I agree with @titustortoiseturtle1999, but nice job on colouring the different sections to allow us to differentiate the different sections.

titustortoiseturtle1999 · 2021-10-28T01:10:56Z

+The Sequence Diagram [below](./next_command_sequence_diagram.png) illustrates within the `Command` component for the `execute(ui,investigation,sceneList)` method call of the `NextCommand` class.
+
+
+![Sequence diagram for execute(ui,investigation,sceneList) method call of NextCommand](./next_command_sequence_diagram.png)


For sequence diagrams, I think there should be a block for each method. eg. When execute(ui, investigation, sceneList) is called should be the start of a block.

I think activations bars are optional, but as some of your other diagrams included it, you may want to standardize it.

titustortoiseturtle1999 · 2021-10-28T01:13:34Z

+  * [Suspect Component](#Suspect-component)
+* [Implementation](#Implementation)

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


I feel that there could be a section here to cover the target user and the value proposition of the product, as well as maybe a brief overview of what it does.

titustortoiseturtle1999 · 2021-10-28T01:18:05Z

+Then read the file and store the information into array list using ```ArrayList<String> content = file.readFile()```.
+Eventually, edit the content and rewrite to data file using ```file.rewriteFile(content)```
+
+###Taking Notes For Specified Scene


This paragraph feels a little wordy, and the length makes it disinteresting to the reader. You could consider reformatting it into something more digestible.

kelvneo · 2021-10-28T02:27:07Z

+## Design

+### Architecture
+![High Level Architectural design](./main_architecture.png)


You may want to change the return arrows to dotted arrows by using <..

Perhaps you can also add a : to all your class names, like :Duke or :Ui.

I think the returning arrows should be dotted instead of a solid line

Perhaps add a : to the name of the classes too.

Is this diagram needed here? As it is repeated again below at line 53.

kelvneo · 2021-10-28T02:29:20Z

+Given below is a quick overview of main components and how they interact with each other.

-{Describe the value proposition: what problem does it solve?}
+###Main components of the architecture


You may want to add a space after your ### to allow GitHub pages to render properly

Adding a space after ### would allow GitHub to render the header correctly.

kelvneo · 2021-10-28T02:36:17Z

+The Sequence Diagram [below](./next_command_sequence_diagram.png) illustrates within the `Command` component for the `execute(ui,investigation,sceneList)` method call of the `NextCommand` class.
+
+
+![Sequence diagram for execute(ui,investigation,sceneList) method call of NextCommand](./next_command_sequence_diagram.png)


I think activations bars are optional, but as some of your other diagrams included it, you may want to standardize it.

kelvneo · 2021-10-28T02:38:25Z

+The Sequence Diagram [below](./next_command_sequence_diagram.png) illustrates within the `Command` component for the `execute(ui,investigation,sceneList)` method call of the `NextCommand` class.
+
+
+![Sequence diagram for execute(ui,investigation,sceneList) method call of NextCommand](./next_command_sequence_diagram.png)


I agree with @titustortoiseturtle1999, but nice job on colouring the different sections to allow us to differentiate the different sections.

RemusTeo

Noticed some errors with the headers, as well as some potential mistakes in the sequence diagrams.

RemusTeo · 2021-10-28T02:06:24Z

+Given below is a quick overview of main components and how they interact with each other.

-{Describe the value proposition: what problem does it solve?}
+###Main components of the architecture


Need to add a space between ### and Main, or else markdown will not generate header

RemusTeo · 2021-10-28T02:16:08Z

+
+The Sequence Diagram below shows how the components interact with each other for the scenario where the user issues the command `/next`.
+
+![High Level Sequence Diagram for main architecture](./main_architecture.png)


Is this image supposed to be here or in line 23? Because I notice that it is the same exact image main_architecture.png.

RemusTeo · 2021-10-28T02:20:01Z

+
+**How the architecture components interact with each other**
+
+The Sequence Diagram below shows how the components interact with each other for the scenario where the user issues the command `/next`.


If the image is a sequence diagram shouldn't there be activation bars?
Also note the returning arrows, I think it should be dotted lines as well.

RemusTeo · 2021-10-28T02:26:46Z

+Then read the file and store the information into array list using ```ArrayList<String> content = file.readFile()```.
+Eventually, edit the content and rewrite to data file using ```file.rewriteFile(content)```
+
+###Taking Notes For Specified Scene


Need to add space between ### and Taking, or else markdown will not generate header.

RemusTeo · 2021-10-28T02:31:22Z

+  * The first investigation scene has a `SuspectList` containing one name, "Father", 
+and four clues within its corresponding `Suspect` class.
+
+![](Suspect.png)


Since there is a method call for new Suspect(), does this mean that it should be using a constructor box?

I agree with @RemusTeo. A constructor box should be added here as well.

markuslyq

Few rendering errors and a few errors with some of the sequence diagrams as well.

markuslyq · 2021-10-28T05:15:30Z

+## Content Page
+* [Architecture](#Architecture)
+* [Main Components of the Architecture](#Main-components-of-the-architecture)
+  * [Parser Component](#Parser-component)
+  * [Note Component](#Note-component)
+  * [UI Component](#UI-component)
+  * [Command Component](#Command-component)
+  * [Investigation Component](#Investigation-component)
+  * [Scene Component](#Scene-component)
+  * [Storage Component](#Storage-component)
+  * [Suspect Component](#Suspect-component)
+* [Implementation](#Implementation)


Perhaps add the appropriate links to correct the navigation as currently, it does not navigate to the correct sub-header.

markuslyq · 2021-10-28T05:20:37Z

+## Design

+### Architecture
+![High Level Architectural design](./main_architecture.png)


I think the returning arrows should be dotted instead of a solid line

markuslyq · 2021-10-28T05:21:09Z

+## Design

+### Architecture
+![High Level Architectural design](./main_architecture.png)


Perhaps add a : to the name of the classes too.

markuslyq · 2021-10-28T05:22:13Z

+## Design

+### Architecture
+![High Level Architectural design](./main_architecture.png)


Is this diagram needed here? As it is repeated again below at line 53.

markuslyq · 2021-10-28T05:24:24Z

+Given below is a quick overview of main components and how they interact with each other.

-{Describe the value proposition: what problem does it solve?}
+###Main components of the architecture


Adding a space after ### would allow GitHub to render the header correctly.

markuslyq · 2021-10-28T05:28:36Z

+`Parser` component for the `getCommandFromUser("/next")` API call.
+
+![Parser design](./ParserUML.png)
+The class diagram below shows how the parser interacts with the other classes


I think a line indentation would make it neater.

If Parser is calling new NextCommand() shouldn't the arrow go to the :NextCommand box, with a constructor below?

markuslyq · 2021-10-28T05:34:07Z

+- Print messages to terminal depending on the scene.
+- Print corresponding output to terminal according to input command.
+
+[UML diagram for Ui](./UiUML.png)


Is there supposed to be an image here? As the image link is broken and would it be better if you just include the image in the DG itself instead of having a link for it?

markuslyq · 2021-10-28T05:37:36Z

+  * The first investigation scene has a `SuspectList` containing one name, "Father", 
+and four clues within its corresponding `Suspect` class.
+
+![](Suspect.png)


I agree with @RemusTeo. A constructor box should be added here as well.

markuslyq · 2021-10-28T05:42:31Z

+See below for example.
+- The introduction scene shows the introductory message to the user.
+- The investigation scene asks the user either investigate a suspect or look into a clue.
+![](Scene.png)


The image here is rather blurry and it makes the words unreadable.

markuslyq · 2021-10-28T05:46:31Z

+At first ```Storage file = new Storage("name.txt")```, initialise the ```Storage``` class type with the name of the file. 
+Then ```file.checkPath()``` will check for existing data file and creates a new data file if the path ```./Data/name.txt```is invalid.
+Then read the file and store the information into array list using ```ArrayList<String> content = file.readFile()```.
+Eventually, edit the content and rewrite to data file using ```file.rewriteFile(content)```


Perhaps a diagram can be included here to allow visualisation of how the storage component works.

LS-Young

Maybe this diagram is a bit messy?

LS-Young

Is this diagram a bit messy?

LS-Young

Is coloring helpful for the division of the sections?

LS-Young

Is suspectName, suspect, name, image, description and clueToAdd necessary here?

LS-Young

Should this diagram be divided into two, so that it can be easier to read?

Implement narrative lines command

Improve ending gameplay

…into branch_note_newest

…r input (from word -> int)

Update both ending narratives

Update investigation to fix bug

update DG

Update PPP

Mf pp v3

Changed javadocs in mf ppp

Bug fix

Update PPP, UG, DG

Update PPP

Pp pupdates

Update PPP

Fix end scene bug

Update about us and UG

…tch-1 Update DeveloperGuide.md

APZH reviewed Oct 27, 2021

View reviewed changes

titustortoiseturtle1999 reviewed Oct 28, 2021

View reviewed changes

kelvneo reviewed Oct 28, 2021

View reviewed changes

RemusTeo reviewed Oct 28, 2021

View reviewed changes

markuslyq reviewed Oct 28, 2021

View reviewed changes

LS-Young reviewed Oct 29, 2021

View reviewed changes

serenakemono and others added 19 commits October 30, 2021 03:04

Merge pull request #131 from arcturusz/narrative-lines-command

d0f10a6

Implement narrative lines command

Merge pull request #132 from arcturusz/ending-scenes

10f8a0d

Improve ending gameplay

Update both ending narratives

6971bd7

add InvalidNote exception & easier way to select option

31b84b4

Merge branch 'master' of https://github.com/AY2122S1-CS2113-T14-1/tp …

96fba55

…into branch_note_newest

Update some instructions and ending

4f3cc38

Add note search exception and note command exception, simplfy the use…

0028d96

…r input (from word -> int)

Fix code syntax

b05cc6b

Syntax fix

8bc6024

Update investigation to fix bug

a3c4eca

Merge pull request #134 from WU-LUOYU-SERENA/branch-v2.1-ending-fix

a320a2a

Update both ending narratives

Merge remote-tracking branch 'origin' into update_investigation

75f1489

Combined exceptions

c512ba2

Syntax fix

39000a8

Update investigation

09916f5

Fix checkstyle not working

2a91932

Update fix intro scene bug

73b9c17

Add try-catch for getSuspectNameFromIndex

6e069a3

Merge pull request #136 from MaifengNg/update_investigation

08bbea6

Update investigation to fix bug

MaifengNg and others added 30 commits November 7, 2021 22:16

Update remove redundant code in SceneListTest

c0a41ef

Update add test for Command Class

4f676ff

Merge branch 'master' into mf_pp_v3

228029d

Fix merge conflict

61e4902

Fix merge conflict

9f7d2e3

Merge pull request #260 from ShaoYurui/master

16e2ef8

update DG

Merge pull request #261 from peng-217/branch_update_DG_UG

491dafd

Update PPP

Merge pull request #262 from MaifengNg/mf_pp_v3

b5fa4f5

Mf pp v3

Changed javadocs in mf ppp

fb08535

Merge pull request #263 from MaifengNg/mf_ppp_v4

d154d22

Changed javadocs in mf ppp

Update PPP, UG, DG

331a1a5

Bug fix

39ded29

syntax fix

656aaee

minor fix

338cf53

Merge pull request #265 from peng-217/branch_update_DG_UG

81d0f06

Bug fix

Project management, other contributions, bolding

2a1047b

Merge pull request #264 from ShaoYurui/master

6eaddec

Update PPP, UG, DG

Update PPP

6867974

Merge pull request #266 from ShaoYurui/master

cb7a9af

Update PPP

Updates to PPP again

70fed81

Merge pull request #267 from Herrekt/PPPupdates

739b38d

Pp pupdates

Update PPP

4327fb1

Merge pull request #268 from peng-217/branch_update_DG_UG

8bef885

Update PPP

Fix end scene bug

b03e501

Merge pull request #269 from arcturusz/ending-scenes

2b0e759

Fix end scene bug

Update about us and UG

feba1ec

Merge pull request #270 from WU-LUOYU-SERENA/master

f521bdf

Update about us and UG

Update DeveloperGuide.md

aa9c3ce

Update DeveloperGuide.md

c0296d1

Merge pull request #271 from AY2122S1-CS2113-T14-1/WU-LUOYU-SERENA-pa…

3823311

…tch-1 Update DeveloperGuide.md

		Here’s a (partial) class diagram of the `Command` component:


		![(partial class) diagram of Command component](./Command_Class_Diagram.png)

		The Sequence Diagram [below](./next_command_sequence_diagram.png) illustrates within the `Command` component for the `execute(ui,investigation,sceneList)` method call of the `NextCommand` class.


		![Sequence diagram for execute(ui,investigation,sceneList) method call of NextCommand](./next_command_sequence_diagram.png)


		The Sequence Diagram below shows how the components interact with each other for the scenario where the user issues the command `/next`.

		![High Level Sequence Diagram for main architecture](./main_architecture.png)


		How the architecture components interact with each other

		The Sequence Diagram below shows how the components interact with each other for the scenario where the user issues the command `/next`.

Uh oh!

Conversation

serenakemono commented Sep 30, 2021

Uh oh!

APZH left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RemusTeo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markuslyq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment