Skip to content

fix/spell_delay #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AdamPhilipSmith wants to merge 5 commits into
base: menu
Choose a base branch
from
Open

fix/spell_delay #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b002b9
Reduced SPELL-DELAY to 15
AdamPhilipSmith Sep 20, 2019
764dce8
Reduced SPELL-DELAY to 15
AdamPhilipSmith Sep 20, 2019
e2d5fcd
Revert "Reduced SPELL-DELAY to 15"
Sep 20, 2019
ddb7f17
My attempt at making a fluid interface. Seems to work but I've had to…
AdamPhilipSmith Sep 21, 2019
e582ae0
Merge branch 'menu' into fix/spell_delay
Veradux Sep 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions src/main/java/com/worldwarofants/game/arch/console/ConsoleOutputHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
public class ConsoleOutputHandler{

private static final long SPELL_DELAY = 15;
private Object ConsoleOutputHandler;
Comment thread
AdamPhilipSmith marked this conversation as resolved.

private void ConsoleOutputHandler(){
//this class is intended to be static and not instanced.
Expand All @@ -13,7 +14,13 @@ private void ConsoleOutputHandler(){
/**
* output the provided string in one line, character by character
*/
public static void spell(String input) {

public ConsoleOutputHandler newOutput(){
ConsoleOutputHandler = new ConsoleOutputHandler();
return this;

}
public ConsoleOutputHandler spell(String input) {
for(int i = 0; i < input.length(); i++) {
System.out.print(input.charAt(i));
try {
Expand All @@ -24,17 +31,28 @@ public static void spell(String input) {
Thread.currentThread().interrupt();

@Veradux Veradux Sep 22, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Do you think that interrupting here is correct? What happened with the SPELL_DELAY? I can't see it being used anywhere.

@AdamPhilipSmith AdamPhilipSmith Sep 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm a bit confused. I didn't write 'Thread.currentThread().interrupt();', it was already there.

Is the SPELL-DELAY not used when the 'spell' method is called in the New Game View?

@Veradux Veradux Sep 23, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

That is the idea. The delay needs to be used in the spell method, to delay every character that is being displayed, to simulate a typing effect. Interrupt() there is a mistake, you need to replace it with something that will achieve that typing effect.

Read a little bit about what threads are in programming, and how to do some basic operations with them. Hint: delaying execution of code.

@AdamPhilipSmith AdamPhilipSmith Sep 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok thanks, I'll look into it. I'm just letting you know that I did not put Interrupt() there. I did not write any of that code, so i didn't want to go about deleting it.

@Veradux Veradux Sep 23, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah I can see that you didn't do it. I'm not sure why it's there though, it doesn't make much sense. So we should probably fix it.

@liam027 liam027 Sep 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey this hasn't changed since we last reviewed it? It's there to handle the Interrupted exception if thrown by the .sleep above.

@Veradux Veradux Sep 23, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Oh my god thanks for making me look into it. Github had folded the code for me and I totally missed the sleep.

@AdamPhilipSmith You don't have to do anything about this, it's working, I'm just blind.

@liam027 How does does interrupting the thread handle the exception? Isn't the exception happening because the thread was already interrupted?

@liam027 liam027 Sep 23, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From my limited understanding of exception handling and threads, it goes like this: if Interrupted while sleeping the thread gets an Interrupted flag and throws the exception. Calling interrupt() on it actually clears (in this case I think toggles) the flag and it is nolonger set to "interrupted" and will just continue what it was doing.

@Veradux Veradux Sep 23, 2019 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Your understanding is correct. But isn't that the same as just ignoring the exception? I'm not sure that anything changes, with or without that line of code.

I don't think that flag is ever checked internally, if I recall correctly. The interrupt flag is there for people to use it to handle such interruptions.

For example, if you're saving the game in a save file, and the thread gets interrupted, you should probably delete the half completed file before closing the thread, if the file is gonna be useless without it being fully finished.

Basically, the interruption mechanism is there to allow you to gracefully finish your threads halfway through, to avoid the risk of your code breaking in weird ways.

}
}
return this;
}
/**
* output the provided string in one line
*/
public static void post(String input) {
System.out.println(input);
public ConsoleOutputHandler post(String input) {
System.out.println(input);
return this;
}
/**
* output a line break
*/
public static void lineBreak() {
System.out.println();
public ConsoleOutputHandler lineBreak() {
System.out.println();
return this;
}

public void print(){
try {
finalize();

@Veradux Veradux Sep 22, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This was a slight misunderstanding. :D Calling the method finalize() is in most cases is very bad. You can read about it here. https://docs.oracle.com/javase/10/docs/api/java/lang/Object.html#finalize()

What I meant in general is to do any final steps, if there are any, in the process.

I dunno what that could be in this particular case. But for example, you could store all of the strings sent through post() in one variable, and on print() you can print it all at once, to sliiightly improve performance, because system.out.println can be considered a slower method than others.

Think about it first though! ;) This is a very cool coding exercise, because whenever spell() is called, it will mess up the order of the messages, if you store all of the posts and print them at once in the end.

} catch (Throwable throwable) {
throwable.printStackTrace();
}
}
}
18 changes: 11 additions & 7 deletions src/main/java/com/worldwarofants/game/module/newgame/NewGameView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ public NewGameView(NewGameViewModel viewModel) {
public void renderNewGameScreen() {
String startGamePrompt = String.format("* Start Game - %s ", NewGameCommandHandler.COMMAND_START_NEW_GAME);
String backPrompt = String.format("* Back - %s ", NewGameCommandHandler.COMMAND_BACK);
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.spell("-- GAME SETUP --");
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.post(startGamePrompt);
ConsoleOutputHandler.post(backPrompt);
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.post("Please enter your command:");

ConsoleOutputHandler console = new ConsoleOutputHandler();

@Veradux Veradux Sep 22, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you try to make it so that you don't have to write the keyword new? And to be able to just say ConsoleOutputHandler.newOutput().something().something()

@AdamPhilipSmith AdamPhilipSmith Sep 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

so yeah, this is what I meant when I said I know some of this stuff I did was kind of a work around haha. I'm yet to fully get my head around static and non-static in Java. I cannot seem to do what you suggest without making the newOutput() method static. However, if I do make it static then I can no longer use ConsoleOutputhundler as a return value in it. This was the only way I could find to get it to work...

@Veradux Veradux Sep 23, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There's nothing wrong with making the method static. That is what we have to do, to get that fluent interface. The starting method has to be static, otherwise it can't be called without making an object.

What is stopping you from using ConsoleOutputhundler as a return value?

@AdamPhilipSmith AdamPhilipSmith Sep 23, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As soon as I make the newOutput() method static it comes up with an error on the return value saying it cannot be referenced. I think I've fixed it though by creating a new ConsoleOutputHandler inside the method.

@Veradux Veradux Sep 23, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yup, that's the goal!

console.newOutput()
.lineBreak()
.spell("-- GAME SETUP --")
.lineBreak()
.post(startGamePrompt)
.post(backPrompt)
.lineBreak()
.post("Please enter your command:")
.print();
}
}
18 changes: 11 additions & 7 deletions src/main/java/com/worldwarofants/game/module/startmenu/StartMenuView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ public StartMenuView(StartMenuViewModel viewModel) {
public void renderStartMenuScreen() {
String newGamePrompt = String.format("* New Game - %s ", StartMenuCommandHandler.COMMAND_NEW_GAME);
String exitPrompt = String.format("* Exit - %s ", StartMenuCommandHandler.COMMAND_EXIT_GAME);
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.spell("-- WORLD WAR OF ANTS --");
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.post(newGamePrompt);
ConsoleOutputHandler.post(exitPrompt);
ConsoleOutputHandler.lineBreak();
ConsoleOutputHandler.post("Please enter your command:");

ConsoleOutputHandler console = new ConsoleOutputHandler();
console.newOutput()
.lineBreak()
.spell("-- WORLD WAR OF ANTS --")
.lineBreak()
.post(newGamePrompt)
.post(exitPrompt)
.lineBreak()
.post("Please enter your command:")
.print();
}
}