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

PTY output not shown without GUI activity #53

Closed
lihop opened this issue Aug 4, 2022 · 6 comments · Fixed by #54
Closed

PTY output not shown without GUI activity #53

lihop opened this issue Aug 4, 2022 · 6 comments · Fixed by #54
Labels
bug Something isn't working

Comments

@lihop
Copy link
Owner

lihop commented Aug 4, 2022

I also encountered another problem, but I don't know if it can be related to this one.

With the same setup (scene with Terminal,Pty and other input nodes) if I execute a command with sudo the terminal screen do not show the command output, to force a refresh I need to press a key or click on other input field to switch focus.

The same command executed without sudo works as expected, the screen show the output.

ping www.google.com works
sudo ping www.google.com doesn't work

Originally posted by @ConteZero in #51 (comment)

@lihop
Copy link
Owner Author

lihop commented Aug 5, 2022

@ConteZero I am not able to reproduce this issue in the test scene:

issue

However, I faced a similar issue when developing the integrated terminal EditorPlugin.

When making the integrated terminal for editor I noticed that the output would not update unless moving the cursor to focus on another panel or after some other type of gui activity (e.g. button click, scroll, cursor blink, etc.). The output from PTY is only updated by calling poll() in _process():

func _process(_delta):
if _pipe:
_pipe.poll()

In editor, _process() is not continuously called unless the editor setting 'update continuously' is set to on:

2022-08-05-113704_753x187_scrot

However, this issue affected all command output regardless of whether it had sudo or not:

issue2

My workaround was to add a timer to regularly call poll() on the underlying pipe:

# In editor _process is not called continuously unless the "Update Continuously"
# editor setting is enabled. This setting is disabled by default and uses 100%
# of one core when enabled, so best to leave it off and use a timer instead.
add_child(timer)
timer.wait_time = 0.025
timer.connect("timeout", self, "_poll")
timer.start()
func _poll():
if pty and pty.has_method("get_master"):
pty.get_master().poll()
update()

I now notice that _physics_process() is called continuously in editor, so I will consider moving the poll from _process() to _physics_process() or adding a process_mode property (like Timer has) to choose between the two.

Could you try changing func _process(_delta) in addons/godot_xterm/nodes/pty/unix/pty_unix.gd to func _physics_process(_delta), or force a regular update by calling pty.get_master().poll() with a timer?

Also, on the off chance that the issue isn't related to the PTY node, but rather the update functions of the Terminal node, you could try setting the update_mode property of Terminal to All (although this will have a slight performance penalty).

If none of these suggestions fix the problem, then I would be interested in more details about your scene setup and operating system version/environment to try and reproduce the issue and figure out why sudo commands might affect the output differently.

@lihop lihop added the bug Something isn't working label Aug 5, 2022
@lihop lihop changed the title Output not shown for sudo commands Output not shown for sudo commands without GUI activity Aug 5, 2022
@ConteZero
Copy link

After doing same code changes to my project now I have the same update problem with and without sudo (update_mode is set to All), probably the differences between the two modes were caused by something wrong with my code.

Luckily your suggestion solves the problem, I've added to my terminal.gd:

func _on_PTY_data_received(data):
#	parsing 'data' code
	_poll()


func _poll():
	if visible and pty and pty.has_method("get_master"): 
		pty.get_master().poll() 
		update()

Thanks for your help.

lihop added a commit that referenced this issue Aug 7, 2022
Ensures a redraw is requested after writing to terminal, otherwise
terminal will not be updated if there are no other redraw requests.

Fixes #53.
@lihop
Copy link
Owner Author

lihop commented Aug 7, 2022

After digging a bit deeper, I have found that simply adding update() at the end of terminal's write function fixes the issue I outlined above, and removes the need for a timer with the editor plugin.

@ConteZero Would you be able to undo the changes you made above and, in terminal.gd, change this code:

func write(data) -> void:
	assert(data is String or data is PoolByteArray)

	# Will be cleared when _flush() is called after VisualServer emits the "frame_pre_draw" signal.
	_buffer.push_back(data)

to this:

func write(data) -> void:
	assert(data is String or data is PoolByteArray)

	# Will be cleared when _flush() is called after VisualServer emits the "frame_pre_draw" signal.
	_buffer.push_back(data)
	update()

and let me know if that also fixes the problem?

@lihop lihop changed the title Output not shown for sudo commands without GUI activity PTY output not shown without GUI activity Aug 7, 2022
@ConteZero
Copy link

I did various tests using _poll(), initially seemed to have solved the problem, instead I found that sometimes some data was not written on the terminal screen.

For now your last solution seems to work well, the only drawback is that it slowdown the execution when the terminal is not visible.
I've changed it to:

func write(data) -> void:
	assert(data is String or data is PoolByteArray)

	# Will be cleared when _flush() is called after VisualServer emits the "frame_pre_draw" signal.
	_buffer.push_back(data)
	if visible:
		update()

This change avoid the slowdown but I don't know if it's right and if it can have unwanted side effects.

@lihop
Copy link
Owner Author

lihop commented Aug 7, 2022

For now your last solution seems to work well, the only drawback is that it slowdown the execution when the terminal is not visible.

@ConteZero Could you try the last solution but with the terminal's update_mode set to the default of Auto rather than All? I'm curious to know if it will still slowdown execution.

Otherwise, I can't think of any unwanted side effect that calling update() only if visible would have.

@ConteZero
Copy link

There are no speed differences between update_mode Auto and All.

lihop added a commit that referenced this issue Aug 7, 2022
Requests a redraw after writing to terminal if it is visible, otherwise
terminal will not be updated if there are no other redraw requests.

Fixes #53.
@lihop lihop closed this as completed in #54 Aug 7, 2022
@lihop lihop closed this as completed in 041e7a4 Aug 7, 2022
lihop added a commit that referenced this issue Aug 21, 2022
This method was never officially documented and used only as a workaround for
issue #53. It also returns an instance of the undocumented and scruffily
implemented Pipe class that I would prefer to keep internal.

Now that #53 has been fixed, this method can be removed from the unofficial
public API, but deprecate it just in case. If someone was using it then it
is still possible (although not supported) to access the `_pipe` property of
`_pty_native`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants