[GH-717] Add Skills section and DB associations for Configurator app (#…

…721)

* mix phx.gen.html CurseOfMirra.Configuration Mechanic

* Add inputs for mechanic references

* Add Arena config fields to Skill and Mechanic

* Configurator app able to create/edit Skill and Mechanic

* Remove unnecesary migration

* Handle nested inputs for mechanics and showing them

* Formatting and credo checks

* Add a required tag to skill.name and mechanic.type in UI

* Add relationship to Character from Skill

* Add game_id to Skill

* Increase layout max-width to allow for big tables

* Minor UI fixes

* Put Skill inside Character

* Formatting

* Send skills as part of character configuration

* Formatting

* Add Skill.type to enable grouping for character assignment

* Handle skills configuration as part of characters and change in format

* Fix tests

* Fix speeds due to outdated config and use the proper key for skill durations

* Fix pools. Add missing activation_delay and effects to skills in seeds

* Fix teleport. Fix pattern-match for teleport mechanic

* Fix Valtimer's basic. on_explode_mechanics were a map of mechanics now is a map of only 1 mechanic

* Fix Valtimer skills, ulti and basic were flipped. Update uma's basic movement behavior

* Add on_explode_mechanics handling and association in mechanics

* Add on_collide_effects association and embedded schema for mechanics

* Fix on_explode_mechanics modal and forms. Capitalize and remove underscores from labels

* Remove commented code

* Restore wrongly deleted mechanic handlers in code

* Improve label texts and remove inspect call

* Delete unused function

* Add on_replace: :update for dash_skill in Character

* Rename skills and mechanic configs migration

* Update Jason.Encoder fields for character schema

* Handle skill deletion when it belongs to a Character

* Add nilify_all on_delete option for skill_id association in mechanics

* Format elixir code

* Encode characters with custom functions and not using Jason's

* Delete wrongly pushed character controller

* Fix leap pattern-match in calculate_duration/4

* Add skill settings to home page

---------

Co-authored-by: Nicolas Sanchez <sanchez.nicolas96@gmail.com>
Co-authored-by: agustinesco <agustinesco@outlook.es>

apps/arena/lib/arena/configuration.ex

@@ -13,12 +13,11 @@ defmodule Arena.Configuration do
       |> File.read()
 
     config = Jason.decode!(config_json, [{:keys, :atoms}])
-    skills = parse_skills_config(config.skills)
-    characters = parse_characters_config(get_characters_config(), skills)
+    characters = parse_characters_config(get_characters_config())
     client_config = get_client_config()
     game_config = get_game_configuration()
 
-    %{config | skills: skills}
+    config
     |> Map.put(:characters, characters)
     |> Map.put(:game, game_config)
     |> Map.put(:client_config, client_config)
@@ -52,10 +51,16 @@ defmodule Arena.Configuration do
     Jason.decode!(payload.body, [{:keys, :atoms}])
   end
 
-  defp parse_skills_config(skills_config) do
-    Enum.reduce(skills_config, [], fn skill_config, skills ->
-      skill = parse_skill_config(skill_config)
-      [skill | skills]
+  defp parse_characters_config(characters) do
+    Enum.map(characters, fn character ->
+      character_skills = %{
+        "1" => parse_skill_config(character.basic_skill),
+        "2" => parse_skill_config(character.ultimate_skill),
+        "3" => parse_skill_config(character.dash_skill)
+      }
+
+      Map.put(character, :skills, character_skills)
+      |> Map.drop([:basic_skill, :ultimate_skill, :dash_skill])
     end)
   end
 
@@ -89,53 +94,32 @@ defmodule Arena.Configuration do
     end)
   end
 
-  defp parse_mechanic_config(mechanic) when map_size(mechanic) > 1 do
-    raise "Config has mechanic with 2 values: #{inspect(mechanic)}"
+  defp parse_mechanic_config(nil) do
+    nil
   end
 
   defp parse_mechanic_config(mechanic) do
-    Map.to_list(mechanic)
-    |> Enum.map(&parse_mechanic_fields/1)
-    |> hd()
-  end
-
-  defp parse_mechanic_fields({:leap, attrs}) do
-    {:leap, %{attrs | on_arrival_mechanic: parse_mechanic_config(attrs.on_arrival_mechanic)}}
-  end
-
-  defp parse_mechanic_fields({name, %{on_explode_mechanics: on_explode_mechanics} = attrs}) do
-    {name,
-     %{
-       attrs
-       | on_explode_mechanics: parse_mechanic_config(on_explode_mechanics)
-     }}
-  end
-
-  defp parse_mechanic_fields(mechanic) do
-    mechanic
-  end
-
-  defp parse_characters_config(characters, config_skills) do
-    Enum.map(characters, fn character ->
-      character_skills =
-        Enum.map(character.skills, fn {skill_key, skill_name} ->
-          skill = find_skill!(skill_name, config_skills)
-          {:erlang.atom_to_binary(skill_key), skill}
-        end)
-        |> Map.new()
-
-      %{character | skills: character_skills}
-    end)
-  end
-
-  defp find_skill!(skill_name, skills) do
-    skill = Enum.find(skills, fn skill -> skill.name == skill_name end)
-
-    ## This is a sanity check when loading the config
-    if skill == nil do
-      raise "Skill #{inspect(skill_name)} does not exist in config"
-    else
-      skill
-    end
+    ## Why do we even need this? Well it happens that some of our fields are represented
+    ## as Decimal. To prevent precision loss this struct its converted to a string of the float
+    ## which should be read back and converted to Decimal
+    ## The not so small problem we have is that our code expects floats so we still need to parse
+    ## the strings, but end up with regular floats
+    %{
+      mechanic
+      | angle_between: maybe_to_float(mechanic.angle_between),
+        move_by: maybe_to_float(mechanic.move_by),
+        radius: maybe_to_float(mechanic.radius),
+        range: maybe_to_float(mechanic.range),
+        speed: maybe_to_float(mechanic.speed),
+        on_arrival_mechanic: parse_mechanic_config(mechanic.on_arrival_mechanic),
+        on_explode_mechanics: parse_mechanics_config(mechanic.on_explode_mechanics)
+    }
+  end
+
+  defp maybe_to_float(nil), do: nil
+
+  defp maybe_to_float(float_string) do
+    {float, ""} = Float.parse(float_string)
+    float
   end
 end

apps/arena/lib/arena/game/player.ex

@@ -246,7 +246,7 @@ defmodule Arena.Game.Player do
   # This is a messy solution to get a mechanic result before actually running the mechanic since the client needed the
   # position in which the player will spawn when the skill start and not when we actually execute the teleport
   # this is also optimistic since we assume the destination will be always available
-  defp maybe_add_destination(action, game_state, player, skill_direction, %{mechanics: [{:teleport, teleport}]}) do
+  defp maybe_add_destination(action, game_state, player, skill_direction, %{mechanics: [%{type: "teleport"} = teleport]}) do
     target_position = %{
       x: player.position.x + skill_direction.x * teleport.range,
       y: player.position.y + skill_direction.y * teleport.range
@@ -454,7 +454,7 @@ defmodule Arena.Game.Player do
   ## so to simplify my life an executive decision was made to take thas as a fact
   ## When the time comes to have more than one mechanic per skill this function will need to be refactored, good thing
   ## is that it will crash here so not something we can ignore
-  defp calculate_duration(%{mechanics: [{:leap, leap}]}, position, direction, auto_aim?) do
+  defp calculate_duration(%{mechanics: [%{type: "leap"} = leap]}, position, direction, auto_aim?) do
     ## TODO: Cap target_position to leap.range
     direction = Skill.maybe_multiply_by_range(direction, auto_aim?, leap.range)
 

apps/arena/lib/arena/game/skill.ex

@@ -13,7 +13,12 @@ defmodule Arena.Game.Skill do
     end)
   end
 
-  def do_mechanic(game_state, entity, {:circle_hit, circle_hit}, %{skill_direction: skill_direction} = _skill_params) do
+  def do_mechanic(
+        game_state,
+        entity,
+        %{type: "circle_hit"} = circle_hit,
+        %{skill_direction: skill_direction} = _skill_params
+      ) do
     circle_center_position = get_position_with_offset(entity.position, skill_direction, circle_hit.offset)
     circular_damage_area = Entities.make_circular_area(entity.id, circle_center_position, circle_hit.range)
 
@@ -63,7 +68,12 @@ defmodule Arena.Game.Skill do
     |> maybe_move_player(entity, circle_hit[:move_by])
   end
 
-  def do_mechanic(game_state, entity, {:cone_hit, cone_hit}, %{skill_direction: skill_direction} = _skill_params) do
+  def do_mechanic(
+        game_state,
+        entity,
+        %{type: "cone_hit"} = cone_hit,
+        %{skill_direction: skill_direction} = _skill_params
+      ) do
     triangle_points =
       Physics.calculate_triangle_vertices(
         entity.position,
@@ -117,9 +127,9 @@ defmodule Arena.Game.Skill do
     |> maybe_move_player(entity, cone_hit[:move_by])
   end
 
-  def do_mechanic(game_state, entity, {:multi_cone_hit, multi_cone_hit}, skill_params) do
+  def do_mechanic(game_state, entity, %{type: "multi_cone_hit"} = multi_cone_hit, skill_params) do
     Enum.each(1..(multi_cone_hit.amount - 1), fn i ->
-      mechanic = {:cone_hit, multi_cone_hit}
+      mechanic = %{multi_cone_hit | type: "cone_hit"}
 
       Process.send_after(
         self(),
@@ -128,12 +138,12 @@ defmodule Arena.Game.Skill do
       )
     end)
 
-    do_mechanic(game_state, entity, {:cone_hit, multi_cone_hit}, skill_params)
+    do_mechanic(game_state, entity, %{multi_cone_hit | type: "cone_hit"}, skill_params)
   end
 
-  def do_mechanic(game_state, entity, {:multi_circle_hit, multi_circle_hit}, skill_params) do
+  def do_mechanic(game_state, entity, %{type: "multi_circle_hit"} = multi_circle_hit, skill_params) do
     Enum.each(1..(multi_circle_hit.amount - 1), fn i ->
-      mechanic = {:circle_hit, multi_circle_hit}
+      mechanic = %{multi_circle_hit | type: "circle_hit"}
 
       Process.send_after(
         self(),
@@ -142,16 +152,16 @@ defmodule Arena.Game.Skill do
       )
     end)
 
-    do_mechanic(game_state, entity, {:circle_hit, multi_circle_hit}, skill_params)
+    do_mechanic(game_state, entity, %{multi_circle_hit | type: "circle_hit"}, skill_params)
   end
 
   def do_mechanic(
         game_state,
         entity,
-        {:dash, %{speed: speed, duration: duration}},
+        %{type: "dash", speed: speed, duration_ms: duration_ms},
         %{skill_direction: skill_direction} = _skill_params
       ) do
-    Process.send_after(self(), {:stop_dash, entity.id, entity.aditional_info.base_speed}, duration)
+    Process.send_after(self(), {:stop_dash, entity.id, entity.aditional_info.base_speed}, duration_ms)
 
     ## Modifying base_speed rather than speed because effects will reset the speed on game tick
     ## by modifying base_speed we ensure that the dash speed is kept as expected
@@ -167,15 +177,15 @@ defmodule Arena.Game.Skill do
     %{game_state | players: players}
   end
 
-  def do_mechanic(game_state, entity, {:repeated_shot, repeated_shot}, skill_params) do
+  def do_mechanic(game_state, entity, %{type: "repeated_shot"} = repeated_shot, skill_params) do
     remaining_amount = repeated_shot.amount - 1
 
     if remaining_amount > 0 do
       repeated_shot = Map.put(repeated_shot, :amount, remaining_amount)
 
       Process.send_after(
         self(),
-        {:trigger_mechanic, entity.id, {:repeated_shot, repeated_shot}, skill_params},
+        {:trigger_mechanic, entity.id, repeated_shot, skill_params},
         repeated_shot.interval_ms
       )
     end
@@ -205,7 +215,12 @@ defmodule Arena.Game.Skill do
     |> put_in([:projectiles, projectile.id], projectile)
   end
 
-  def do_mechanic(game_state, entity, {:multi_shoot, multishot}, %{skill_direction: skill_direction} = skill_params) do
+  def do_mechanic(
+        game_state,
+        entity,
+        %{type: "multi_shoot"} = multishot,
+        %{skill_direction: skill_direction} = skill_params
+      ) do
     entity_player_owner = get_entity_player_owner(game_state, entity)
 
     calculate_angle_directions(multishot.amount, multishot.angle_between, skill_direction)
@@ -234,7 +249,12 @@ defmodule Arena.Game.Skill do
     end)
   end
 
-  def do_mechanic(game_state, entity, {:simple_shoot, simple_shoot}, %{skill_direction: skill_direction} = skill_params) do
+  def do_mechanic(
+        game_state,
+        entity,
+        %{type: "simple_shoot"} = simple_shoot,
+        %{skill_direction: skill_direction} = skill_params
+      ) do
     last_id = game_state.last_id + 1
     entity_player_owner = get_entity_player_owner(game_state, entity)
 
@@ -259,7 +279,7 @@ defmodule Arena.Game.Skill do
     |> put_in([:projectiles, projectile.id], projectile)
   end
 
-  def do_mechanic(game_state, entity, {:leap, leap}, %{execution_duration: execution_duration}) do
+  def do_mechanic(game_state, entity, %{type: "leap"} = leap, %{execution_duration: execution_duration}) do
     Process.send_after(
       self(),
       {:stop_leap, entity.id, entity.aditional_info.base_speed, leap.on_arrival_mechanic},
@@ -278,7 +298,7 @@ defmodule Arena.Game.Skill do
     put_in(game_state, [:players, player.id], player)
   end
 
-  def do_mechanic(game_state, entity, {:teleport, _teleport}, %{skill_destination: skill_destination}) do
+  def do_mechanic(game_state, entity, %{type: "teleport"}, %{skill_destination: skill_destination}) do
     entity =
       entity
       |> Map.put(:aditional_info, entity.aditional_info)
@@ -287,15 +307,10 @@ defmodule Arena.Game.Skill do
     put_in(game_state, [:players, entity.id], entity)
   end
 
-  def do_mechanic(game_state, player, {:spawn_pool, pool_params}, skill_params) do
-    %{
-      skill_direction: skill_direction,
-      auto_aim?: auto_aim?
-    } = skill_params
-
+  def do_mechanic(game_state, player, %{type: "spawn_pool"} = pool_params, skill_params) do
     last_id = game_state.last_id + 1
 
-    skill_direction = maybe_multiply_by_range(skill_direction, auto_aim?, pool_params.range)
+    skill_direction = maybe_multiply_by_range(skill_params.skill_direction, skill_params.auto_aim?, pool_params.range)
 
     target_position = %{
       x: player.position.x + skill_direction.x,

apps/arena/lib/arena/game_socket_handler.ex

@@ -188,15 +188,14 @@ defmodule Arena.GameSocketHandler do
   defp to_broadcast_skill({key, skill}) do
     ## TODO: This will break once a skill has more than 1 mechanic, until then
     ##   we can use this "shortcut" and deal with it when the time comes
-    [{_mechanic, params}] = skill.mechanics
-
-    extra_params =
-      %{
-        targetting_radius: params[:radius],
-        targetting_angle: params[:angle],
-        targetting_range: params[:range],
-        targetting_offset: params[:offset] || params[:projectile_offset]
-      }
+    [mechanic] = skill.mechanics
+
+    extra_params = %{
+      targetting_radius: mechanic[:radius],
+      targetting_angle: mechanic[:angle],
+      targetting_range: mechanic[:range],
+      targetting_offset: mechanic[:offset] || mechanic[:projectile_offset]
+    }
 
     {key, Map.merge(skill, extra_params)}
   end

apps/configurator/lib/configurator_web/controllers/character_controller.ex

@@ -11,15 +11,13 @@ defmodule ConfiguratorWeb.CharacterController do
 
   def new(conn, _params) do
     changeset = Ecto.Changeset.change(%Character{})
-    render(conn, :new, changeset: changeset)
+    skills = get_curse_skills_by_type()
+    render(conn, :new, changeset: changeset, skills: skills)
   end
 
   def create(conn, %{"character" => character_params}) do
-    # TODO This should be removed once we have the skills relationship, issue: https://github.com/lambdaclass/mirra_backend/issues/717
-    skills = Jason.decode!(character_params["skills"])
-
     character_params =
-      Map.put(character_params, "skills", skills)
+      character_params
       |> Map.put("game_id", GameBackend.Utils.get_game_id(:curse_of_mirra))
       |> Map.put("faction", "curse")
 
@@ -30,7 +28,8 @@ defmodule ConfiguratorWeb.CharacterController do
         |> redirect(to: ~p"/characters/#{character}")
 
       {:error, %Ecto.Changeset{} = changeset} ->
-        render(conn, :new, changeset: changeset)
+        skills = get_curse_skills_by_type()
+        render(conn, :new, changeset: changeset, skills: skills)
     end
   end
 
@@ -42,13 +41,11 @@ defmodule ConfiguratorWeb.CharacterController do
   def edit(conn, %{"id" => id}) do
     character = Characters.get_character(id)
     changeset = Ecto.Changeset.change(character)
-    render(conn, :edit, character: character, changeset: changeset)
+    skills = get_curse_skills_by_type()
+    render(conn, :edit, character: character, changeset: changeset, skills: skills)
   end
 
   def update(conn, %{"id" => id, "character" => character_params}) do
-    # TODO This should be removed once we have the skills relationship, issue: https://github.com/lambdaclass/mirra_backend/issues/717
-    skills = Jason.decode!(character_params["skills"])
-    character_params = Map.put(character_params, "skills", skills)
     character = Characters.get_character(id)
 
     case Characters.update_character(character, character_params) do
@@ -58,7 +55,8 @@ defmodule ConfiguratorWeb.CharacterController do
         |> redirect(to: ~p"/characters/#{character}")
 
       {:error, %Ecto.Changeset{} = changeset} ->
-        render(conn, :edit, character: character, changeset: changeset)
+        skills = get_curse_skills_by_type()
+        render(conn, :edit, character: character, changeset: changeset, skills: skills)
     end
   end
 
@@ -70,4 +68,9 @@ defmodule ConfiguratorWeb.CharacterController do
     |> put_flash(:success, "Character deleted successfully.")
     |> redirect(to: ~p"/characters")
   end
+
+  defp get_curse_skills_by_type() do
+    GameBackend.Units.Skills.list_curse_skills()
+    |> Enum.group_by(& &1.type)
+  end
 end