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-309] add valtimer teleport #305

Merged
merged 29 commits into from
Apr 8, 2024
Merged

[GH-309] add valtimer teleport #305

merged 29 commits into from
Apr 8, 2024

Conversation

agustinesco
Copy link
Contributor

@agustinesco agustinesco commented Feb 22, 2024

Closes #309

Implement teleport mechanic

The teleport mechanic will move the player in the same instant when it's executed, the player should not be vulnerable while this is occurring

Testing

to test this pr play a match with valtimer with this client branch and use the teleport ability, you can also go to this link and press the "P" key

player
|> Physics.move_entity_to_position(target_position, game_state.external_wall)
|> Map.put(:aditional_info, player.aditional_info)
|> maybe_make_invinsible(teleport)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure I understand, they idea is to give you 300 ms (current config) of damage immunity after you teleport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, te idea is to give inmunity while you're teleporting, also we should adjust the time of the inmunity later on when we have something solid in the client

Copy link
Contributor

@AminArria AminArria Feb 22, 2024

Choose a reason for hiding this comment

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

Then why do you need the immunity? Teleportation is instant, how could someone damage you while teleporting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a hacky solution for how the teleport will be represented in the front, they will make the character model to go under the floor, walk down there and then get up on the final destinations. Thinking about it, maybe we don't even need a new case for do_mechanic because we could simply make it a dash and call it a teleport, the thing is that the character won't be visible, so the invisibility is needed

Copy link
Contributor

Choose a reason for hiding this comment

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

JAJAJAJA that explains it

🤔 mmm honestly don't like any of those hacks... would it be possible for them to dissapear and appear the model? We could, in theory, remove the player from game_state.players and bring it back say 30 ms laters (or whatever we configure), essentially disappear the player for some time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm i'm not entirelly sure, maybe we could talk with @saugon about this

@agustinesco agustinesco changed the title add valtimer teleport [GH-309] add valtimer teleport Mar 11, 2024
{
"teleport": {
"range": 1000,
"duration_ms": 300
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should include the radius of the ability so we can displayed it in the client. Or maybe we can use the hitbox radius in the client to display it.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm i think the radius and the range are the same in the context of the teleport, we could simply rename this

Copy link
Contributor

@BertovDev BertovDev Mar 26, 2024

Choose a reason for hiding this comment

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

I do not think that is gonna work because we use the range and area to draw different indicators in the client. I mean with the range we draw the outer circle area and with the radius the inner circle area.

Screenshot 2024-03-26 at 16 15 00

What I suggest is that maybe we can use the hitbox of the character to display the inner area since it indicates where the character body is gonna be placed. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooooh right! yeah we should use the body size for the inner circle indicator

@agustinesco agustinesco marked this pull request as ready for review March 27, 2024 17:26
@@ -362,4 +382,16 @@ defmodule Arena.Game.Player do
player
end
end

defp maybe_make_invinsible(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why "invisible" instead of "invulnerable"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was the name given by the item implementation, i just s̶t̶o̶l̶e̶ use that in this case

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I just realized it says "invinsible", not "invisible" as in not visible. There's a typo there though, it should be "invincible"!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right! i didn't noticed it either lol fixed!

@@ -362,4 +382,16 @@ defmodule Arena.Game.Player do
player
end
end

defp maybe_make_invinsible(
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I just realized it says "invinsible", not "invisible" as in not visible. There's a typo there though, it should be "invincible"!

Copy link
Contributor

@lotuuu lotuuu left a comment

Choose a reason for hiding this comment

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

Tested on the client and everything worked fine!

action_name = skill_key_execution_action(skill_key)
action =
%{
action: skill_key_execution_action(skill_key),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think action_name would be a better name here? At first sight it looks like action contains an action, as a sort of compound entity, which is not what you're trying to represent here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that makes sense , so we can avoid this abomination action.action

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually i think this is a bigger change than just renaming this, because we need to change the player action everywhere. This change was only something to avoid adding multiple parameters to the add_action function, if you think this make things worse we can revert it

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, let's leave it like this then

@AngieDutra AngieDutra merged commit 0b2c244 into main Apr 8, 2024
1 check passed
@AngieDutra AngieDutra deleted the add-valtimer-tp branch April 8, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add valtimer warp teleport
9 participants