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

Set Default Song in Levelbuilder #25155

Merged
merged 9 commits into from Oct 2, 2018
Merged

Set Default Song in Levelbuilder #25155

merged 9 commits into from Oct 2, 2018

Conversation

epeach
Copy link

@epeach epeach commented Oct 1, 2018

Builds on: #25058 and #25081

*Adds dropdown in levelbuilder to set the default song
*Song Selection UI is auto-populated with this value
*Note: The dropdown is populated from a manually curated list of songs in dancelab.rb I left a todo in there for myself as a future step will be the use the song library manifest to populate these values.

Has no change on user-experience, so not behind the 'songSelector' flag.

Upcoming PRs:
Use the song chosen in the Song Selector UI when the project is played
Remove set song block

@@ -70,6 +70,11 @@
#plusPreloadAssetList
%i.fa.fa-plus-circle

.field
= f.label :default_song, 'Dance Lab Only - Default Song'
= f.select :default_song, options_for_select(@level.class.hoc_songs, @level.default_song)
Copy link
Member

Choose a reason for hiding this comment

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

better to wrap this .field with - if @level.is_a? Dancelab.

the next time we add a setting specific to Dancelab, we should really extract a new _dancelab.html.haml partial, and then refer to it like this:

= render partial: 'levels/editors/frequency_analysis', locals: {f: f} if @level.is_a? FrequencyAnalysis

# Overriden by different level types.
def self.hoc_songs
end

Copy link
Member

Choose a reason for hiding this comment

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

ok to omit this method definition once you are ensuring that it is only called for Dancelab.

@@ -79,6 +79,7 @@ class Blockly < Level
preload_asset_list
skip_autosave
skip_run_save
default_song
Copy link
Contributor

Choose a reason for hiding this comment

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

Will other app types use this in the future, or should this go in gamelab.rb?

@@ -534,6 +534,10 @@ def blockly_options
level_options[:sharedFunctions] = nil # TODO: handle non-standard pools
end

if @level.is_a? Dancelab
level_options[:defaultSong] = @level.default_song
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this — the following ensures that all level properties are turned into options on appOptions.level:

# Add all level view options to the level_options hash
level_options.merge! level_overrides.camelize_keys
app_options.merge! view_options.camelize_keys

@epeach epeach merged commit cb49816 into staging Oct 2, 2018
@epeach epeach deleted the levelbuilder-default-song branch October 12, 2018 16:50
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.

None yet

3 participants