Fix: Prevent game crash on sink double-click#56
Fix: Prevent game crash on sink double-click#56Itx-Psycho0 wants to merge 3 commits intoknative-extensions:mainfrom
Conversation
|
@Itx-Psycho0: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
|
|
Welcome @Itx-Psycho0! It looks like this is your first PR to knative-extensions/educational-game 🎉 |
|
I have signed the CLA and verified the fix locally. The double-click crash issue is resolved. Summary of Changes: Added safety check for conveyerInd array bounds. Verified with the attached video proof. Ready for your review! |
prajjwalyd
left a comment
There was a problem hiding this comment.
Thanks for this fix @Itx-Psycho0! The check prevents the crash.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Itx-Psycho0, prajjwalyd The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
One small concern, though: |
|
@prajjwalyd I'll review this PR by today, the script is common to all levels, so I have to check it. |
|
@Itx-Psycho0 This is not working for multi sink level, let's say I click once on blue, and twice on red, and after that I can't click on green, as it is considering 2 belts for red and not allowing the 3rd one for green. FYI, In the multi sink level, the player need not click all the sinks, the player can click less as well. The events are duplicated accordingly for the number of sinks connected. |
|
/lgtm cancel |
…ion bug - Added dynamic conveyor creation for MultiSink level (3+ conveyors) - Maintained safety check for BasicEventFlow to prevent crash - Fixed level.gd to initialize before scene change (not after) - Resolves issue where MultiSink failed during full game playthrough - Addresses maintainer feedback about connection limits Fixes knative-extensions#55
|
Hi @ankitajana21, Thank you for the detailed feedback! You were absolutely right - my initial fix limited MultiSink to 3 connections. I've now implemented a comprehensive solution that fixes both the crash AND allows unlimited connections. Root CauseAfter debugging, I found TWO bugs:
SolutionConveyerController.gd - Smart Conveyor Managementif conveyerInd >= conveyer.size():
// Detect scene by conveyor count
if conveyer.size() >= 3:
// MultiSink: Create new conveyors dynamically
var new_conveyor = conveyer[0].duplicate()
get_tree().current_scene.add_child(new_conveyor)
conveyer.append(new_conveyor)
else:
// BasicEventFlow: Stop to prevent crash
returnlevel.gd - Fixed Scene Transition// Initialize BEFORE changing scene (not after)
ConveyerController.initialise()
get_tree().change_scene_to_file(next_level_path)Testing ResultsBasicEventFlow: Double-click no longer crashes Video DemonstrationScreen.Recording.2026-01-29.102927.mp4The fix now properly addresses both the crash prevention AND allows the flexible multi-sink behavior you described. Ready for review! |
Changes
transfer_boxfunction.conveyerIndis within the valid range of theconveyerarray before accessing it./kind bug
Fixes #55
Verification (Video)
issue55.mp4
Release Note