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

Making spikes have perks for the placer #1135

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

farooqkz
Copy link
Contributor

LV/LW said it works on his machine so I didn't test further.

mods/ctf/ctf_map/ctf_traps.lua Outdated Show resolved Hide resolved
mods/ctf/ctf_map/ctf_traps.lua Outdated Show resolved Hide resolved
mods/ctf/ctf_map/ctf_traps.lua Outdated Show resolved Hide resolved
mods/ctf/ctf_map/ctf_traps.lua Outdated Show resolved Hide resolved
mods/ctf/ctf_map/ctf_traps.lua Outdated Show resolved Hide resolved
mods/ctf/ctf_map/ctf_traps.lua Show resolved Hide resolved
Copy link
Member

@LoneWolfHT LoneWolfHT left a comment

Choose a reason for hiding this comment

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

It stopped working on the test server for some reason. I'll fix this and merge...

@farooqkz
Copy link
Contributor Author

It stopped working on the test server for some reason. I'll fix this and merge...

Which test server?

@src4026
Copy link
Contributor

src4026 commented Apr 23, 2023

It stopped working on the test server for some reason. I'll fix this and merge...

Which test server?

LV had ran a test server a few days ago testing the spike perk (and another PR + Polls).

Copy link
Member

@LoneWolfHT LoneWolfHT left a comment

Choose a reason for hiding this comment

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

Spikes should hit through medic immunity. Let me know if you can't reproduce that though

@LoneWolfHT LoneWolfHT removed the Post-approval step ⭐ Something remains to be done before merge label Jul 29, 2023
@farooqkz
Copy link
Contributor Author

farooqkz commented Aug 11, 2023

Spikes should hit through medic immunity. Let me know if you can't reproduce that though

I can't reproduce.

Correction: I was testing on old code.

@farooqkz
Copy link
Contributor Author

Spikes should hit through medic immunity. Let me know if you can't reproduce that though

Should the spikes do "full damage" when medics have got immunity? And should the damage be on the behalf of the placer?

@LoneWolfHT
Copy link
Member

Should the spikes do "full damage" when medics have got immunity? And should the damage be on the behalf of the placer?

Yeah, if the placer is online, otherwise just deal damage

@farooqkz
Copy link
Contributor Author

Spikes work through medic immunity, too. This PR is ready for another review and possible test. I'm using my server to launch a test server. Will post details on Discord.

Comment on lines +121 to +124
return -7, false
elseif placer then
player:punch(placer, 1, { fleshy = 7, spike = 1})
return -7, false
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the return value heal any damage dealt?

Copy link
Member

Choose a reason for hiding this comment

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

In fact, I don't think this code is reachable unless they aren't damaged by a spike

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, I don't think this code is reachable unless they aren't damaged by a spike

how so? Are you talking about L121 or L123-124?

Copy link
Member

Choose a reason for hiding this comment

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

110: if team and reason.node == string.format("ctf_map:spike_%s", team) then

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.

3 participants