-
Notifications
You must be signed in to change notification settings - Fork 4
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
[GH-1027] Add game mode params per map #1032
Conversation
ad64d9a
to
373afb5
Compare
…ode in forms & controllers
…. Also add it's behavior
7989201
to
a3ea317
Compare
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.
Great work! I've left some comments to take into account in the future
@derive {Jason.Encoder, only: [:name, :zone_enabled, :bots_enabled, :match_duration_ms, :respawn_time_ms]} | ||
|
||
schema "game_mode_configurations" do | ||
field(:name, Ecto.Enum, values: [:battle_royale, :deathmatch]) |
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'm not sure about this restriction here; maybe we could have another field for this (like a game mode type). However, we can leave it as it is
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.
Oh the field name is debatable. But being type or name, it could be an Ecto.Enum as it is or something else. Tell me your thoughts so we improve it if it doesn't work!
schema "map_mode_params" do | ||
field(:amount_of_players, :integer) | ||
field(:deleted_at, :naive_datetime) | ||
embeds_many(:solo_initial_positions, Position, on_replace: :delete) |
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 would add a validation here saying that the initial positions length should be equal to the amount of players
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.
Noted. I'll create an issue with all your feedback ❤️
</span> | ||
</div> | ||
|
||
<.inputs_for :let={fp} field={f[:map_mode_params]}> |
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.
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.
Will do in a separate PR. Thanks!
</span> | ||
</div> | ||
|
||
<div class="flex items-center justify-between gap-6"> |
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.
We can add an X button within every nested form, so we can delete an specific configuration and not the last one
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.
Alright. Let me know if it's done somewhere already so I can implement it faster 🚀
The error wasn't at creation, it was at editing (update) time. |
Motivation
We can't support multiple configs for multiple game modes and/or maps.
Closes #1027
Summary of changes
How to test it?
Open the configurator (localhost:4100) and interact with every game mode form.
Disclaimer⚠️
This is the 1st step to achieve the game modes - maps configurations. This just adds the CRUD and forms, in a next iteration this will be used to serve the config to the client.
Checklist