-
Notifications
You must be signed in to change notification settings - Fork 20
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
updates to make reloading of the blockcode work in game #280
Conversation
If I run this in Godot 4.3 I get an output error and the main scene not playing:
|
I was having problems with that line in the original PR here: (For some reason the automated However, my additional PR doesn't call this function because it uses |
Just as a comment, the bad formatting for the Due to the incompatibility of the 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. |
@dylanmccall would you like to take a look at this? |
There was a problem hiding this 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).
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) |
There was a problem hiding this comment.
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 :)
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. |
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:
MainPanel.switch_block_code_node(_block_code)
MainPanel.script_window_requested
signal (after changing the text in the ShowScriptButton)is_block_code_editable()
to return trueundo_redo = null
_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.