Skip to content
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.

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

Make stdin / stdout a game console #7266

Merged
merged 23 commits into from
Mar 23, 2018

Conversation

IntelOrca
Copy link
Contributor

@IntelOrca IntelOrca commented Mar 11, 2018

This PR refactors the in game console into three classes:

  • A base class containing the commands.
  • An implementation for stdin / stdout (allowing interactive console in headless mode)
  • An implementation for the in game console (which is now in openrct2-ui)

One small issue is that anytime there is a printf or log in headless mode, it will mess up the current prompt on stdout. Fixing this would require replacing all stdout calls with our own method which overwrites the current prompt and writes a new prompt with the current statement. I am not going to worry about this for now.

I originally wrote the interactive console for the plug-in branch, but it will be a long time before that goes into the game. So for now this should allow more interim console commands to be added and used in headless servers.

For the interactive console, I have used linenoise which comes as a single .hpp that we can include.

@AaronVanGeffen AaronVanGeffen added this to the v0.2.0 milestone Mar 11, 2018
@IntelOrca IntelOrca force-pushed the feature/interactive-stdinout branch 5 times, most recently from 48dfc38 to e9278f5 Compare March 13, 2018 21:47
@IntelOrca IntelOrca closed this Mar 16, 2018
@IntelOrca IntelOrca deleted the feature/interactive-stdinout branch March 16, 2018 21:56
@IntelOrca IntelOrca restored the feature/interactive-stdinout branch March 16, 2018 22:03
@IntelOrca IntelOrca reopened this Mar 16, 2018
@IntelOrca IntelOrca force-pushed the feature/interactive-stdinout branch from 9b41f20 to 1ae950f Compare March 17, 2018 18:38
{
if (_consoleHistoryCount >= CONSOLE_HISTORY_SIZE) {
for (sint32 i = 0; i < _consoleHistoryCount - 1; i++)
memcpy(_consoleHistory[i], _consoleHistory[i + 1], CONSOLE_INPUT_SIZE);
Copy link
Member

Choose a reason for hiding this comment

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

Happy to see the in-game console move to the UI subproject. However, this seems to be undoing the refactoring from #7064. Specifically, the console has been using a deque for its history since that PR.

Copy link
Contributor Author

@IntelOrca IntelOrca Mar 18, 2018

Choose a reason for hiding this comment

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

I haven't changed anything about the way it works, this is a straight move. This code still exists in that PR.

Copy link
Member

Choose a reason for hiding this comment

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

It appears you're right — sorry. I wonder why these instances of memcpy weren't replaced at the time, then… Others were replaced with deque::push_back and the likes.

Copy link
Member

@Broxzier Broxzier left a comment

Choose a reason for hiding this comment

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

Tested with a few commands, and seems to work nicely.

case CONSOLE_INPUT_HISTORY_PREVIOUS:
if (_consoleHistoryIndex > 0) {
_consoleHistoryIndex--;
memcpy(_consoleCurrentLine, _consoleHistory[_consoleHistoryIndex], 256);
Copy link
Member

Choose a reason for hiding this comment

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

256 = CONSOLE_INPUT_SIZE

case CONSOLE_INPUT_HISTORY_NEXT:
if (_consoleHistoryIndex < _consoleHistoryCount - 1) {
_consoleHistoryIndex++;
memcpy(_consoleCurrentLine, _consoleHistory[_consoleHistoryIndex], 256);
Copy link
Member

Choose a reason for hiding this comment

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

256 = CONSOLE_INPUT_SIZE

@IntelOrca IntelOrca force-pushed the feature/interactive-stdinout branch from 1f7426f to e4af39b Compare March 20, 2018 16:49
@janisozaur
Copy link
Member

Typing hide closes the client.

I fear this is something we won't be able to address unless we move to readline, but the console right now feels very limited: line editing boils down to most basic cursor movement: you can't even do per-word moving of caret. That said it is still very welcome addition over no console whatsoever.

@janisozaur
Copy link
Member

Opening the cli client without any park specified just loops and doesn't allow for any interaction, even though it displays initial prompt.

@IntelOrca
Copy link
Contributor Author

I fear this is something we won't be able to address unless we move to readline, but the console right now feels very limited: line editing boils down to most basic cursor movement: you can't even do per-word moving of caret. That said it is still very welcome addition over no console whatsoever.

Please improve it if you have time.

Opening the cli client without any park specified just loops and doesn't allow for any interaction, even though it displays initial prompt.

I am aware of the infinite loop, but this is not an issue with this PR. It is getting stuck in peep update.

Copy link
Member

@janisozaur janisozaur left a comment

Choose a reason for hiding this comment

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

The code looks mostly good, but there are some issues still:

  1. exiting causes a segfault. ~Context calls network_close -> gfx_invalidate_screen -> context_get_height, but we are in ~Context already.
  2. SIGINT is not handled properly: it should discard current input and possibly produce a message to press it again or do ^D or type exit

@IntelOrca IntelOrca force-pushed the feature/interactive-stdinout branch from 901cfb3 to 4babfa2 Compare March 22, 2018 20:29
@janisozaur
Copy link
Member

$ ./openrct2-cli host /some/park.sv6
Unable to open 'host'

@IntelOrca
Copy link
Contributor Author

➜  bin git:(feature/interactive-stdinout) ✗ ./openrct2-cli host /data/rct1/Scenarios/sc3.sc4
ERROR[../src/openrct2/network/TcpSocket.cpp:146 (Listen)]: IPV6_V6ONLY failed. 92
Ready for clients...

@janisozaur
Copy link
Member

Whoops, I had networking disabled 😊

@janisozaur
Copy link
Member

There's only segfault-at-exit to address now.

@janisozaur
Copy link
Member

I can't test it now, but if the segfault is gone, I'm happy for this to be merged, as long as CIs are green.

Separate out closing of connection into a new method so only that is called when closing the game which then means gfx_invalidate_screen is not called.
@IntelOrca IntelOrca force-pushed the feature/interactive-stdinout branch from 39831c5 to a6d03b3 Compare March 23, 2018 21:54
@IntelOrca IntelOrca merged commit a913fd2 into OpenRCT2:develop Mar 23, 2018
@IntelOrca IntelOrca deleted the feature/interactive-stdinout branch May 13, 2018 11:15
janisozaur added a commit that referenced this pull request Jun 10, 2018
- Feature: [#1417] Allow saving track designs for flat rides.
- Feature: [#1675] Auto-rotate shops to face footpaths.
- Feature: [#3473] Add button in ride window's maintainance tab to refurbish the ride.
- Feature: [#6510] Ability to select edges or a row of tiles by holding down Ctrl using the land tool.
- Feature: [#7187] Option for early scenario completion.
- Feature: [#7266] Make headless instances use an interactive terminal with access to the in-game console API.
- Feature: [#7267] Leverage more historical data in Finances window.
- Feature: [#7316] Cheat to allow freezing all staff
- Feature: [#7332] Keyboard shortcuts for view path issues and cutaway view.
- Feature: [#7348] Add large half loops to the Vertical Drop Roller Coaster.
- Feature: [#7459] Allow opening and closing of parks that use no money.
- Feature: [#7579] New horizontal +/- spinner widgets to make adjusting values easier.
- Fix: [#2053] When clearance checks are disabled, a track piece ghost can remove non-ghost track pieces.
- Fix: [#2611] Some objects show (undefined string) instead of a description in Korean.
- Fix: [#3596] Saving parks, landscapes and tracks with a period in the filenames don't get their extension.
- Fix: [#5210] Default system dialog not accessible from saving landscape window.
- Fix: [#6134] Scenarios incorrectly categorised when using Polish version of RCT2.
- Fix: [#6141] CSS50.dat is never loaded.
- Fix: [#6647] Changelog window causes FPS drop.
- Fix: [#6938] Banner do not correctly capitalise non-ASCII characters.
- Fix: [#7176] Mechanics sometimes fall down from rides.
- Fix: [#7303] Visual glitch with virtual floor near map edges.
- Fix: [#7313] Loading an invalid path with openrct2 produces results different than expected.
- Fix: [#7327] Abstract scenery and stations don't get fully See-Through when hiding them (original bug).
- Fix: [#7331] Invention list in scenario editor crashes upon removing previously-enabled ride/stall entries.
- Fix: [#7341] Staff may auto-spawn on guests walking outside of paths.
- Fix: [#7354] Cut-away view does not draw tile elements that have been moved down on the list.
- Fix: [#7358] Peeps and staff entering rides still have the slope speed penalty set.
- Fix: [#7382] Opening the mini-map reverts the size of the land tool to 1x1, regardless of what was selected before.
- Fix: [#7402] Edges of neigbouring footpaths stay connected after removing a path that's underneath a ride entrance.
- Fix: [#7405] Rides can be covered by placing scenery underneath them.
- Fix: [#7418] Staff walk off paths with a connection but no adjacent path.
- Fix: [#7434] Diagonal ride segments cannot be deleted if they are isolated.
- Fix: [#7436] Only the first 32 vehicles of a train can be painted.
- Fix: [#7480] Graphs skip values of 0.
- Fix: [#7505] Game crashes when trying to make path over map edge while having clearance checks disabled.
- Fix: [#7528] In park entrance pricing tab, switching tabs happens on mouse-down instead of mouse-up
- Fix: [#7544] Starting a headless server with no arguments causes the game to freeze.
- Fix: [#7571] Hovering a ride design over scenery or tracks will give tons of money.
- Improved: [#2989] Multiplayer window now changes title when tab changes.
- Improved: [#5339] Change eyedropper icon to actual eyedropper and change cursor to crosshair.
- Improved: [#5832] Resize tile inspector automatically when selecting a tile element.
- Improved: [#6221] The scenario editor's invention list is now resizeable.
- Improved: [#7069] The arbitrary ride type selection dropdown is now sorted orthographically, and has its spinners removed.
- Improved: [#7302] Raising land near the map edge makes the affected area smaller instead of showing an 'off edge map' error.
- Improved: [#7435] Object indexing now supports multi-threading.
- Improved: [#7510] Add horizontal clipping to cut-away view options.
- Improved: [#7531] "Save track design" dropdown now stays open.
- Improved: [#7548] Ctrl-clicking with the tile inspector open now directly selects an element and its tile.
- Improved: [#7555] Allow setting the Twitch API URL, allowing custom API servers.
- Improved: [#7567] Improve the performance of loading parks and the title sequence.
- Improved: [#7577] Allow fine-tuning the virtual floor style.
- Improved: [#7608] The vehicle selection dropdown is now sorted orthographically.
- Improved: [#7613] Resizing the staff window now resizes the name and action columns too.
- Improved: [#7627] Allow scrolling up and down on spinners to change their values.
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.

None yet

5 participants