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

[GH-798] - Configurator Game Modes #855

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft

Conversation

tvillegas98
Copy link
Contributor

@tvillegas98 tvillegas98 commented Aug 12, 2024

Closes #798

Motivation

Add the game mode concept to the configurator. So we can group configurations below a game mode

DISCLAIMER: We're not addressing UI/UX concerns in this PR.

Summary of changes

How to test it?

UI/UX stills rough.

  • Go to CONFIGURATOR_URL/game_modes create a game mode
  • Go to CONFIGURATOR_URL/versions and select a game mode
  • Play a Game and check that everything works as usually

Checklist

  • Tested the changes locally.
  • Reviewed the changes on GitHub, line by line.
  • This change requires new documentation.
    • Documentation has been added/updated.

@tvillegas98 tvillegas98 marked this pull request as ready for review August 13, 2024 19:23
agustinesco
agustinesco previously approved these changes Aug 19, 2024
Copy link
Contributor

@agustinesco agustinesco left a comment

Choose a reason for hiding this comment

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

LGTM!

@tvillegas98 tvillegas98 added the R1 First Review label Aug 21, 2024

def change do
alter table(:game_modes) do
add :description, :text
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to squash this into the other migration AddGameModesTable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. Nice catch!

use Ecto.Migration

def change do
create table(:game_modes) do
Copy link
Contributor

@AminArria AminArria Aug 22, 2024

Choose a reason for hiding this comment

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

Why another table instead of an enum in versions? It feels like a lot of code/complexity to keep track of a string. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point, but I think that marking a version as current in a game mode would be a bit more complex? What if we want to disabled a game mode? 🤔 .
That's a good idea to be honest, but I would to see if this fits our necessities or it is too much 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

How do you envision to that now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R1 First Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Configurator WebApp: Implement GameModes tables
3 participants