-
Notifications
You must be signed in to change notification settings - Fork 479
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
Levelbuilder Clean Up: Playlab edit #35089
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
92999a0
Use the control buttons partial in all places where edit control buttons
dmcavoy a626fd7
Move playlab validation editors into validation area
dmcavoy 21c8b0a
Group together sprites and collisions settings
dmcavoy 3874975
move coordinate_grid_background to cs_in_a editor since its only used…
dmcavoy c7bb70a
Merge branch 'weblab-edit' into playlab-edit
dmcavoy ef7c53c
Merge branch 'weblab-edit' into playlab-edit
dmcavoy File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,32 @@ | ||
| //CS in A stuff | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :use_contract_editor, description: "Use MSM contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :default_num_example_blocks, description: "Default # of example blocks in contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :disable_examples, description: "Hide examples section in contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :examples_required, description: "Each function must have at least 1 example, and examples will be checked when hitting run"} | ||
| %table.contract-editor-config | ||
| %tr | ||
| %th Section | ||
| %th Highlight | ||
| %th Collapse | ||
| -%w(contract examples definition).each_with_index do |section, i| | ||
| %tr | ||
| %td #{i + 1}. #{section.titleize} | ||
| %td= boolean_check_box f, "#{section}_highlight".to_sym | ||
| %td= boolean_check_box f, "#{section}_collapse".to_sym | ||
| %div.collapsed_area_header{data: {toggle: "collapse", target: "#cs_in_a"}} | ||
| CS in A Options | ||
| .collapse{id: "cs_in_a"} | ||
| - if @level.respond_to? :coordinate_grid_background | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :coordinate_grid_background, description: "Coordinate grid background"} | ||
|
|
||
| -if @level.respond_to? :input_output_table | ||
| .field | ||
| = f.label :input_output_table, 'Input/Output Table' | ||
| %p An array of tuples where each tuple is [input, output]. | ||
| %p i.e. The table for f(x) = x - 5 might look like: [ [15, 10], [10, 5], [25, 20] ] | ||
| = f.text_area :input_output_table, placeholder: 'Input/Output', rows: 4, value: @level.input_output_table | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :use_contract_editor, description: "Use MSM contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :default_num_example_blocks, description: "Default # of example blocks in contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :disable_examples, description: "Hide examples section in contract editor"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :examples_required, description: "Each function must have at least 1 example, and examples will be checked when hitting run"} | ||
| %table.contract-editor-config | ||
| %tr | ||
| %th Section | ||
| %th Highlight | ||
| %th Collapse | ||
| -%w(contract examples definition).each_with_index do |section, i| | ||
| %tr | ||
| %td #{i + 1}. #{section.titleize} | ||
| %td= boolean_check_box f, "#{section}_highlight".to_sym | ||
| %td= boolean_check_box f, "#{section}_collapse".to_sym | ||
|
|
||
| -if @level.respond_to? :input_output_table | ||
| .field | ||
| = f.label :input_output_table, 'Input/Output Table' | ||
| %p An array of tuples where each tuple is [input, output]. | ||
| %p i.e. The table for f(x) = x - 5 might look like: [ [15, 10], [10, 5], [25, 20] ] | ||
| = f.text_area :input_output_table, placeholder: 'Input/Output', rows: 4, value: @level.input_output_table |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,60 +1,33 @@ | ||
| - content_for(:head) do | ||
| %script{src: webpack_asset_path('js/levels/editors/_studio.js')} | ||
|
|
||
| .field | ||
| = render partial: 'levels/editors/fields/encrypted_examples', locals: {f: f, level_type: 'playlab'} | ||
| = render partial: 'levels/editors/fields/control_buttons', locals: {f: f} | ||
|
|
||
| .field | ||
| = f.label :custom_game_type, 'Custom game type' | ||
| %p Used to tell playlab we want to run some of our custom onTick logic. | ||
| = f.select :custom_game_type, options_for_select(["", "Big Game", "Rocket Height", "Sam the Bat", "Ninja Cat"], @level.custom_game_type) | ||
| .field | ||
| = f.label :background, 'Default background' | ||
| = f.text_field :background | ||
|
|
||
| %legend.control-legend.collapsed{data: {toggle: "collapse", target: "#sprites-and-collisions"}} | ||
| Sprites and Collisions | ||
|
|
||
| #sprites-and-collisions.in.collapse | ||
| = f.label :first_sprite_index, 'First sprite index' | ||
| %p Integer representing which sprite to use for the first character. Default is 0. | ||
| = f.number_field :first_sprite_index | ||
| .field | ||
|
|
||
| = f.label :protaganist_sprite_index, 'Protaganist sprite index' | ||
| %p Integer of the protaganist character (zero indexed, counting from the top left of the grid). This character must touch all the waypoint flags to complete the level. If no value is set, all sprites can collect flags. | ||
| = f.number_field :protaganist_sprite_index | ||
| .field | ||
| = f.label :timeout_failure_tick, 'Timeout failure tick' | ||
| %p Number of 'ticks' to simulate before marking the level as failed. Default is unlimited. Step speed (above) controls the length of a tick. | ||
| = f.number_field :timeout_failure_tick | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :timeout_after_when_run, description: "Timeout after all blocks run"} | ||
| %p When set, if the only event block that had children is when_run, and those commands are finished executing, don't wait for the timeout. If we have additional event blocks that DO have children, we keeping running until timeoutFailureTick or a success/failure condition is met | ||
| .field | ||
| = f.label :custom_game_type, 'Custom game type' | ||
| %p Used to tell playlab we want to run some of our custom onTick logic. | ||
| = f.select :custom_game_type, options_for_select(["", "Big Game", "Rocket Height", "Sam the Bat", "Ninja Cat"], @level.custom_game_type) | ||
| .field | ||
| = f.label :success_condition, 'Success condition' | ||
| %p Optional JavaScript function to run every tick. If the function ever return true, the level immediately succeeds. | ||
| = f.text_area :success_condition, rows: 4 | ||
| .field | ||
| = f.label :failure_condition, 'Failure condition' | ||
| %p Optional JavaScript function to run every tick. If the function ever return true, the level immediately fails. | ||
| = f.text_area :failure_condition, rows: 4 | ||
| .field | ||
| = f.label :soft_buttons, 'Controls buttons' | ||
| %p | ||
| Select | ||
| %a.select_all{href: '#'} all | ||
| \/ | ||
| %a.select_none{href: '#'} none | ||
| (shift-click or cmd-click to select multiple). Arrow buttons to display below the game canvas. | ||
| = f.collection_select :soft_buttons, soft_button_options, :value, :name, {selected: @level.soft_buttons}, {multiple: true} | ||
| .field | ||
|
|
||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :edge_collisions, description: "Edge collisions"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :projectile_collisions, description: "Projectile collisions"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :allow_sprites_outside_playspace, description: "Allow sprites outside playspace"} | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :sprites_hidden_to_start, description: "Sprites hidden to start"} | ||
| .field | ||
| = f.label :remove_items_when_actor_collides, 'Remove an item (e.g. a stormtrooper) when the actor collides with it' | ||
| = boolean_check_box f, :remove_items_when_actor_collides | ||
| .field | ||
| = f.label :background, 'Default background' | ||
| = f.text_field :background | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :remove_items_when_actor_collides, description: "Remove an item (e.g. a stormtrooper) when the actor collides with it"} | ||
|
|
||
| %div.collapsed_area_header{data: {toggle: "collapse", target: "#cs_in_a"}} | ||
| CS in A Options | ||
| .collapse{id: "cs_in_a"} | ||
| = render partial: 'levels/editors/cs_in_a', locals: {f: f} | ||
| = render partial: 'levels/editors/cs_in_a', locals: {f: f} |
15 changes: 15 additions & 0 deletions
15
dashboard/app/views/levels/editors/fields/_studio_validations.html.haml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| .field | ||
| = f.label :timeout_failure_tick, 'Timeout failure tick' | ||
| %p Number of 'ticks' to simulate before marking the level as failed. Default is unlimited. Step speed (above) controls the length of a tick. | ||
| = f.number_field :timeout_failure_tick | ||
| .field | ||
| = render partial: 'levels/editors/fields/checkboxes', locals: {f: f, field_name: :timeout_after_when_run, description: "Timeout after all blocks run"} | ||
| %p When set, if the only event block that had children is when_run, and those commands are finished executing, don't wait for the timeout. If we have additional event blocks that DO have children, we keeping running until timeoutFailureTick or a success/failure condition is met | ||
| .field | ||
| = f.label :success_condition, 'Success condition' | ||
| %p Optional JavaScript function to run every tick. If the function ever return true, the level immediately succeeds. | ||
| = f.text_area :success_condition, rows: 4 | ||
| .field | ||
| = f.label :failure_condition, 'Failure condition' | ||
| %p Optional JavaScript function to run every tick. If the function ever return true, the level immediately fails. | ||
| = f.text_area :failure_condition, rows: 4 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think it's fine to do here since we're already doing it so much, but the proliferation of these type checks is worrying to me in general. It'd be worth thinking about if there's a better approach that lets us control things on a per-level basis without needing to scatter conditions throughout the code base.
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.
Good call @uponthesun ... I agree , especially when there are multiple dimensions to the checks, such as class types as well as whether the level is a droplet level. here are some ideas:
A simple (but at this point time-consuming) change to the current approach would be to nest all of the partials according to the class structure. so, levels/editors/_applab.html.haml would include levels/editors/_blockly.html.haml, which would include app/views/levels/editors/_all.html.haml (possibly renamed to _level.html.haml). this gives us simplicity, but puts some restrictions on the way we lay out the level edit page. for example, if we have debugger controls which are applab specific and others which are more general, then it would be hard to show those on the same part of the page.
Another idea would be to have one giant template for editing levels, containing a place to display each level edit option. then each level type would say whether it cared about that field, or, ideally, we could check if the level type had the property or column corresponding to the field and use that to decide whether to display it.
it's possible some hybrid approach is possible where we take the simpler approach (1) overall, but in cases where we want to group certain types of controls more tightly, we use approach 2 on just that section/partial.
anyway... since we're 20 PRs down the current path right now I don't think we want to redo this at the moment, but let's plan to follow a more deliberate pattern here when we move to any new UI for level editing. ideally the solution will move to react, leaving most of this code behind.
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.
Thanks for the feedback. My actual plan is to get ride of the all partial soon and move each of these fields into the specific level files once things are more cleaned up and clear what should live where. I know its a little bit of a weird path to get there but its the one I could wrap my head around how to slowly move over instead of huge PRs that might be hard to follow. Anyway hopefully in future PRs you will see an improvement here.
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.
Sounds great Dani. If you go that approach, you could still extract some shared partials as needed. but in general I'm very much in favor of each level editor page having its own partial which pulls in any shared stuff it needs, rather than every level page pulling in the same shared partial which then decides what to pull in based on the level type.