Skip to content

Games on GTOS#133

Merged
bruberu merged 21 commits intomasterfrom
gtosGaming
Oct 5, 2021
Merged

Games on GTOS#133
bruberu merged 21 commits intomasterfrom
gtosGaming

Conversation

@bruberu
Copy link
Copy Markdown
Member

@bruberu bruberu commented Sep 4, 2021

Adds a few games to the GTOS terminal, such as Pong. I plan to create at least 3 for this PR before submitting.

Comment on lines +125 to +129
public Widget setVisibilitySupplier(BooleanSupplier supplier) {
visibilitySupplier = supplier;
return this;
}

Copy link
Copy Markdown
Member

@Yefancy Yefancy Sep 22, 2021

Choose a reason for hiding this comment

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

My advice is not to add this API, it is somewhat coupled. I think it's better to check the game state and update the visible in MazeApp's updateScreen, what do you think? Of course, if you think this interface is really necessary and will be used in future widgets, then let it go. I don't think it's a problemm, just want the abstraction and the API to be cleaner and necessary.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Personally, while I think this could be done, it would require one of two implementations. Either I would loop through the entire array of widgets, and apply the suppliers one-by-one with a variable for each of them, or I would create an instance variable for everything that needed dynamic visibility, and automatically pass in the argument on each cycle. However, in both cases, I would need 11 variables just to implement this for the MazeApp, some of which with very similar names. This would probably be harder to read then just adding a supplier to a widget directly. Also, I will probably be using this in the third game in this project, and it may end up having more uses in the future.

Comment thread src/main/java/gregtech/common/terminal/app/game/maze/MazeApp.java Outdated
app.setGamestate(1);
app.setMazesSolved(0);
MAZE_SIZE = 9;
speed = 25;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe app.speed = 25;? It is recommended to use initApp for initialization

Comment on lines +61 to +68
app.addWidget(new SimpleTextWidget(333 / 2, 232 / 2 - 40, "", 0xFFFFFFFF, () -> "Oh no! You were eaten by the Minotaur!", true).setVisibilitySupplier(() -> app.getGamestate() == 3));
app.addWidget(new SimpleTextWidget(333 / 2, 232 / 2 - 28, "", 0xFFFFFFFF, () -> "You got through " + app.getMazesSolved() + " mazes before losing.", true).setVisibilitySupplier(() -> app.getGamestate() == 3));
app.addWidget(new SimpleTextWidget(333 / 2, 232 / 2 - 16, "", 0xFFFFFFFF, () -> "Try again?", true).setVisibilitySupplier(() -> app.getGamestate() == 3));
app.addWidget(new ClickButtonWidget(323 / 2 - 10, 222 / 2 + 10, 40, 20, "Retry", (clickData -> {
app.setGamestate(1);
app.setMazesSolved(0);
MAZE_SIZE = 9;
speed = 25;
Copy link
Copy Markdown
Member

@Yefancy Yefancy Sep 22, 2021

Choose a reason for hiding this comment

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

Very good game interaction. Maybe you could also try TerminalDialogWidget showInfoDialog (...).setclientSide ().open(). It is good for this kind of work.

@bruberu
Copy link
Copy Markdown
Member Author

bruberu commented Oct 3, 2021

As of now, as there are 3 games added at this point, this PR is ready to be reviewed extensively and merged.

@bruberu bruberu marked this pull request as ready for review October 3, 2021 02:54
@bruberu bruberu requested a review from a team October 3, 2021 02:54
@Yefancy
Copy link
Copy Markdown
Member

Yefancy commented Oct 4, 2021

All games have been played a few times and feel no logical bugs. I think itcan merge, but it's up to you, if you don't have anything else to do. its a good stuff, i really like it. BTW, i like UI of the minesweeper, it would be better if other game have ui as good as minesweeper.

@bruberu
Copy link
Copy Markdown
Member Author

bruberu commented Oct 5, 2021

I think I want to move on to other projects, and I'm glad that you liked it. I will merge now.

@bruberu bruberu merged commit d0bda52 into master Oct 5, 2021
@bruberu bruberu deleted the gtosGaming branch October 28, 2021 01:37
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.

2 participants