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

updates to make reloading of the blockcode work in game #280

Conversation

goatchurchprime
Copy link

This ports across the necessary changes from https://github.com/goatchurchprime/godot-block-coding/tree/blockcodingingame to this branch so it saves and reloads the edited block code back onto the running node.

In particular these were:

  • calling MainPanel.switch_block_code_node(_block_code)
  • connecting the MainPanel.script_window_requested signal (after changing the text in the ShowScriptButton)
  • editing is_block_code_editable() to return true
  • handling the fact that undo_redo = null
  • calling _ready() after the script has loaded.

This version is probably good enough to do some experiments where there is only one BlockCode editor window open in the game.

This feature could be important because makes it possible to implement an introduction to coding that's even friendlier to students since it requires no interaction with the Godot editor.

@manuq
Copy link
Contributor

manuq commented Oct 21, 2024

If I run this in Godot 4.3 I get an output error and the main scene not playing:

Invalid access to property or key 'set_block_code_node' on a base object of type 'Object (BlockEditorContext)'.

@goatchurchprime
Copy link
Author

I was having problems with that line in the original PR here:
https://github.com/endlessm/godot-block-coding/blob/block-code-demo-scene/block_code_demo.gd#L13

(For some reason the automated set_ function wasn't existing, in spite of this automated set definition:
https://github.com/endlessm/godot-block-coding/blob/block-code-demo-scene/addons/block_code/ui/block_editor_context.gd#L8 )

However, my additional PR doesn't call this function because it uses MainPanel.switch_block_code_node(_block_code).

https://github.com/goatchurchprime/godot-block-coding/blob/block-code-demo-scene/block_code_demo.gd#L19

@goatchurchprime
Copy link
Author

Just as a comment, the bad formatting for the if undo_redo: shouldn't matter because it is the wrong way to do it. For now it is just to suppress this class and make it work.

Due to the incompatibility of the UndoRedo and the EditorUndoRedoManager, it probably requires a new object that can wrap both classes and call whichever is appropriate.

This would not be a bad thing, since sharing edits across multiple players is best implemented in through such an UndoRedo system if it acts as a single point of committing. It can even handle clashes where an out-of-order commit from another player can trigger an undo before it is committed.

@manuq
Copy link
Contributor

manuq commented Oct 24, 2024

@dylanmccall would you like to take a look at this?

@dylanmccall dylanmccall self-requested a review October 25, 2024 01:27
Copy link
Contributor

@dylanmccall dylanmccall left a comment

Choose a reason for hiding this comment

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

Thank you so much for picking this branch up and running with it, @goatchurchprime ! It's super exciting seeing that demo scene actually work properly.

Indeed that stuff with UndoRedo is a bit of a pickle. Kind of funny that they're different the way they are, but a wrapper class would probably be helpful for us anyway. (I wonder if it would make sense for that class to expose actions like add_block_code_for_node and deal with the two UndoRedo interfaces behind the scenes? There's a lot of fussy boilerplate when we deal with the undo stack right now).

Anyway, I know this is just going into a feature branch, so I'd be happy to merge this whenever to keep that moving along!

About the linting stuff, we use gdformat with pre-commit, so (with pre-commit installed) nice and easy to get this passing the checks by running pre-commit run -a. (It's a super nice tool! I like to drop little pitches at random).

Comment on lines +26 to +43
var showscriptwindowfirst = true
const ScriptWindow := preload("res://addons/block_code/ui/script_window/script_window.tscn")
func script_window_requested(scriptcontent):
if showscriptwindowfirst:
var script_window = ScriptWindow.instantiate()
script_window.script_content = scriptcontent
add_child(script_window)
await script_window.close_requested
script_window.queue_free()
script_window = null

var block_code_parent = _block_code.get_parent()
var script := GDScript.new()
script.set_source_code(scriptcontent)
script.reload()
block_code_parent.set_script(script)
block_code_parent._ready()
block_code_parent.set_process(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, took me a minute to figure out what we're doing here, then I realized we handle script_window_requested in block_code_plugin.gd so this has to do it all over again. That seems like a bit of a mistake - we're happy spawning dialogs from main_panel.gd in other cases.

(Granted, using EditorInterface.popup_dialog_centered(), which is also going to be a mistake in this case).

As a note for later, might be good moving that whole concern into main_panel.gd, and then we should be able to drop all the stuff with script_window.tscn here.

I suspect in the end we'll want to have a different main_panel.tscn altogether for this, which would probably not have that node chooser option button at the top, and have a Play button instead. But to get there we'd need to break that scene up a bit as well (so we aren't duplicating a bunch of stuff), so I can appreciate that this is working with what we have at the moment. And, uh, indeed good to not refactor a bunch of things in this branch since it's rather behind main :)

@goatchurchprime
Copy link
Author

Okay, I've done the changes to satisfy the linter. If it doesn't do any damage, it could be merged.

As I pointed out, the UndoRedo system is the key to making this system multiplayer, so a wrapper provides the place for such an implementation.

With multiplayer it would be technically possible to have a help button that spawns in a mentor (the terminology used in Resonite) who can un-stick a new programmer stuck on a problem. Unlike mathematical problem solving, problem solving in computer programming is usually held up by a missing piece of knowledge about an available function whose definition make little sense until you hit the problem is it designed to solve. In a classroom setting the teacher can be physically called to the desk of a student who is stuck on a problem and can point to the lines on the terminal while talking through the issue. These interactions are also an opportunity for the teacher to introduce challenges to a student who is ready for it to keep them moving along.

I probably need to keep pushing on with my general purpose multiplayer+voip component into which this system could interface.

@dylanmccall dylanmccall merged commit f6c8698 into endlessm:block-code-demo-scene Oct 30, 2024
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.

3 participants