Skip to content

C4 Rework#1739

Merged
TimGoll merged 27 commits into
TTT-2:masterfrom
MrXonte:C4-rework
Jan 29, 2025
Merged

C4 Rework#1739
TimGoll merged 27 commits into
TTT-2:masterfrom
MrXonte:C4-rework

Conversation

@MrXonte

@MrXonte MrXonte commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

Now scales C4 damage with radius and gives consistent damage through walls regardless of line of sight.
Relates to #1729 and #1730 although the calculation for proper Killzone is not yet included, but the safe and killzones are now 100% correct.

@MrXonte

MrXonte commented Jan 23, 2025

Copy link
Copy Markdown
Contributor Author

im not sure what im doing wrong for the total history to show up, but I sycnched my fork before PR #1738 and it showed up, now I made a branch and its still all there

@MrXonte

MrXonte commented Jan 23, 2025

Copy link
Copy Markdown
Contributor Author

Also, whats wrong with the style check? Cant figure out what the problem here is
image

@TimGoll

TimGoll commented Jan 23, 2025

Copy link
Copy Markdown
Member

im not sure what im doing wrong for the total history to show up, but I sycnched my fork before PR #1738 and it showed up, now I made a branch and its still all there

If you make a new branch AFTER the commits were added, the new branch won't change anything.

I barely work with forks, so my knowledge is spotty. But you probably not only need to sync to the host, but also rebase it, so your commit history is removed in favor of ours (we squash PRs, so your commits are not in our repository)

@TimGoll

TimGoll commented Jan 23, 2025

Copy link
Copy Markdown
Member

Also, whats wrong with the style check? Cant figure out what the problem here is image

@Histalek looks like stylua crashed?

@MrXonte the crash might have happened because you removed the empty line at file end (which always should be there)

image

Comment thread lua/ttt2/libraries/game_effects.lua Outdated
@Histalek

Copy link
Copy Markdown
Member

Also, whats wrong with the style check? Cant figure out what the problem here is image

@Histalek looks like stylua crashed?

@MrXonte the crash might have happened because you removed the empty line at file end (which always should be there)

image

that's not a crash and only stylua reporting a style issue (but that style issue is about non-printable characters, which makes the output you see kinda useless)

and yes you're spot on with the missing newline at the end of the file

Comment thread lua/ttt2/libraries/game_effects.lua Outdated
Comment thread lua/ttt2/libraries/game_effects.lua Outdated
Comment thread lua/ttt2/libraries/game_effects.lua Outdated
Comment thread lua/ttt2/libraries/game_effects.lua Outdated
Comment thread lua/ttt2/libraries/game_effects.lua Outdated
Comment thread gamemodes/terrortown/entities/entities/ttt_c4/shared.lua
@MrXonte MrXonte requested a review from TimGoll January 27, 2025 09:19
@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

If you change something, I'm available for another 30 minutes or so

if its ok with you, I'd set the Outer range to 1500? Heres a comparison:
image
image

or we set it to linear

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

yeah, that looks good and it compensates the old LOS kill I think

@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

image

Also, I'm not sure if this was different before, but C4 is not listed as inflictor. Do you know how it was before?

no idea how it was before, but im doing it the same way the old spheredamage did. It used self for the inflictor, and im parsing self as the inflictor into the new spheredamage, so it should be the same as before
OLD:
image
NEW:
image
image

increased outer radius to 1500
@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

no idea how it was before, but im doing it the same way the old spheredamage did. It used self for the inflictor, and im parsing self as the inflictor into the new spheredamage, so it should be the same as before

Yes I looked at the code as well. I'm confused. I guess it did not work before either because you pass self, which is the c4 ent, but not the c4 weapon.

could you try to pass ents.Create("weapon_ttt_c4") instead of self?

@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

no idea how it was before, but im doing it the same way the old spheredamage did. It used self for the inflictor, and im parsing self as the inflictor into the new spheredamage, so it should be the same as before

Yes I looked at the code as well. I'm confused. I guess it did not work before either because you pass self, which is the c4 ent, but not the c4 weapon.

could you try to pass ents.Create("weapon_ttt_c4") instead of self?

it should also be possible to parse the entity instead of the weapon, but I'm gonna check later

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

it should also be possible to parse the entity instead of the weapon

Actually I don't think so. Looking at the code here makes me believe this never worked:
image

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

If my suggestion fixes it, could you please test it in an old TTT2 version (before your PR) to confirm it was broken there as well? If it was, could you then please add this to the changelog? Thank you!

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

Also, I don't know if this should be part of this PR, but if you changed the kill/damage radius, you should probably also change the UI (marker vision) to this?

proper safe/damage/killzone UI
@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

Also, I don't know if this should be part of this PR, but if you changed the kill/damage radius, you should probably also change the UI (marker vision) to this?

I've included it now since i do think it should be part of the PR. if we are gonna change how it works, we should also make sure the display is (finally) correct

@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

it should also be possible to parse the entity instead of the weapon

Actually I don't think so. Looking at the code here makes me believe this never worked: image

hm this complicates things, but i guess that just means we should finally expand this class to allow for entities to be displayed here? A well defined entity should include all the relevant data we want to display here since we just do a lot of back and forth to print out the entity printname/id anyways?

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

hm this complicates things, but i guess that just means we should finally expand this class to allow for entities to be displayed here? A well defined entity should include all the relevant data we want to display here since we just do a lot of back and forth to print out the entity printname/id anyways?

I have to look into that. In theory that could be an easy change, but I'm not sure if this has any unwanted side effects.

I know I spent some time getting the boomerang to work because it is also a generic entity, not a weapon. I solved this, but I forgot how

@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

hm this complicates things, but i guess that just means we should finally expand this class to allow for entities to be displayed here? A well defined entity should include all the relevant data we want to display here since we just do a lot of back and forth to print out the entity printname/id anyways?

I have to look into that. In theory that could be an easy change, but I'm not sure if this has any unwanted side effects.

I know I spent some time getting the boomerang to work because it is also a generic entity, not a weapon. I solved this, but I forgot how

if your entity has a .ScoreName then this is taken instead if its not a weapon, at least for the kill event.
image
In the corpse its worse, since it only allows for a weapon or nothing at all, but if you parsed an entity here it might also work (or break horribly)
image

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

We have to make TTT3 that breaks compatibility with all that shit, that looks awful :D

I'm not sure what I changed for bodysearch. But I did something to support weapons such as the boomerang

Edit: Found the change: #1382

However, this only fixes missing icons. This doesn't address the fact that C4 isn't registered at all

@MrXonte

MrXonte commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

If my suggestion fixes it, could you please test it in an old TTT2 version (before your PR) to confirm it was broken there as well? If it was, could you then please add this to the changelog? Thank you!

as the placer of the C4, I get the C4 note
image
but definitely no indication that C4 was the cause when it kills someone else
image

@TimGoll

TimGoll commented Jan 27, 2025

Copy link
Copy Markdown
Member

Thank you for investigating this!

Speaking of this: I hate that the C4 note is hardcoded in the TTT2 code base. I have to remove it from there and use the same pipeline that addons use.

@TimGoll

TimGoll commented Jan 28, 2025

Copy link
Copy Markdown
Member

In case you're waiting on me. I'm fine with this as soon as you test the ents.Create("weapon_ttt_c4") suggestion

@TimGoll

TimGoll commented Jan 29, 2025

Copy link
Copy Markdown
Member

this change actually fixed it:

image

For me this PR is done. If you want to investigate registering entites as inflictors, feel free to go ahead in a further PR

@MrXonte

MrXonte commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

In case you're waiting on me. I'm fine with this as soon as you test the ents.Create("weapon_ttt_c4") suggestion

i actually just didn't have time yet, but great that it seems to work now! :D (also Eieruhr C4 xD)

@TimGoll

TimGoll commented Jan 29, 2025

Copy link
Copy Markdown
Member

(also Eieruhr C4 xD)

:D That's for the PietSmiet easteregg pack

FYI, I've got some plans for the rework of the C4 UI I'm currently working on. This will trigger some reworks of the damage again. So don't be confused when I change the parameters again.

@TimGoll TimGoll merged commit a438ec6 into TTT-2:master Jan 29, 2025
@MrXonte

MrXonte commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

(also Eieruhr C4 xD)

:D That's for the PietSmiet easteregg pack

FYI, I've got some plans for the rework of the C4 UI I'm currently working on. This will trigger some reworks of the damage again. So don't be confused when I change the parameters again.

aahhhh i knew i knew it from somewhere xD

the MV UI elements or the use UI? 🤔 Also I think scaling damage with the fuze time would be great. Its a bit weird to only make it harder to defuse since you can just go away from it much more easily if the fuze is longer.

@TimGoll

TimGoll commented Jan 29, 2025

Copy link
Copy Markdown
Member

See #1734, the PR is about the UI and some light rebalancing

If you want to discuss my ideas, it is best to post them there

Histalek pushed a commit to WardenPotato/TTT2 that referenced this pull request Jan 31, 2025
Now scales C4 damage with radius and gives consistent damage through
walls regardless of line of sight.
Relates to TTT-2#1729 and TTT-2#1730 although the calculation for proper Killzone
is not yet included, but the safe and killzones are now 100% correct.

---------

Co-authored-by: Tim Goll <github@timgoll.de>
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