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-852] Configurator: Migrate effects config #856

Merged
merged 20 commits into from
Sep 12, 2024
Merged

Conversation

agustinesco
Copy link
Contributor

@agustinesco agustinesco commented Aug 12, 2024

Motivation

Keep migrating fields from config.json to configurator app

Part of #852

Summary of changes

  • Added embedded schema Effect to be shared across distinct structs
  • Adapted in game effects to work with a field instead of looking for effects in the game config
  • Added show and edit of effects to configurator app

How to test it?

Start a match and everything should keep working as before

Checklist

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

@agustinesco agustinesco marked this pull request as ready for review August 13, 2024 16:07
@agustinesco agustinesco changed the title [GH-852] Migrate effects config [GH-852] Configurator: Migrate effects config Aug 13, 2024
tvillegas98
tvillegas98 previously approved these changes Aug 27, 2024
Copy link
Contributor

@tvillegas98 tvillegas98 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 previously approved these changes Sep 2, 2024
tvillegas98
tvillegas98 previously approved these changes Sep 5, 2024
Copy link
Contributor

@AminArria AminArria left a comment

Choose a reason for hiding this comment

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

Not sure where/how, but the Valtimer dash is broken

Stacktrace

[error] GenServer #PID<0.1554.0> terminating
** (KeyError) key :execute_multiple_times not found in: {:damage_immunity, %{effect_delay_ms: 0, execute_multiple_times: false}}

If you are using the dot syntax, such as map.field, make sure the left-hand side of the dot is a map
    (arena 0.7.1) lib/arena/game/effect.ex:158: anonymous fn/5 in Arena.Game.Effect.apply_effect_mechanic/3
    (stdlib 5.0.2) maps.erl:416: :maps.fold_1/4
    (elixir 1.16.1) lib/enum.ex:2528: Enum."-reduce/3-lists^foldl/2-0-"/3
    (arena 0.7.1) lib/arena/game/effect.ex:143: anonymous fn/2 in Arena.Game.Effect.apply_effect_mechanic_to_entities/1
    (stdlib 5.0.2) maps.erl:416: :maps.fold_1/4
    (arena 0.7.1) lib/arena/game_updater.ex:259: Arena.GameUpdater.handle_info/2
    (stdlib 5.0.2) gen_server.erl:1077: :gen_server.try_handle_info/3
    (stdlib 5.0.2) gen_server.erl:1165: :gen_server.handle_msg/6
    (stdlib 5.0.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

@agustinesco agustinesco marked this pull request as draft September 11, 2024 14:01
@agustinesco agustinesco marked this pull request as ready for review September 11, 2024 17:21
tvillegas98
tvillegas98 previously approved these changes Sep 12, 2024
AminArria
AminArria previously approved these changes Sep 12, 2024
@agustinesco agustinesco dismissed stale reviews from AminArria and tvillegas98 via 7a17a7c September 12, 2024 17:56
@tvillegas98 tvillegas98 merged commit e8cb8d6 into main Sep 12, 2024
1 check passed
@tvillegas98 tvillegas98 deleted the migrate-effects-config branch September 12, 2024 18:04
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.

3 participants