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

Grenades! #68

Merged
merged 53 commits into from
Dec 6, 2024
Merged

Grenades! #68

merged 53 commits into from
Dec 6, 2024

Conversation

legokidlogan
Copy link
Member

Makes some improvements/fixes to the throwable and grenade bases, and adds some new grenades: discombob, impact discombob, cluster, super cluster, curse, and anti-gravity.

Requires CFC-Servers/cfc_ulx_commands#159 to be merged.

@legokidlogan legokidlogan added the enhancement New feature or request label Sep 26, 2024
@legokidlogan legokidlogan self-assigned this Sep 26, 2024
@legokidlogan legokidlogan marked this pull request as draft September 27, 2024 14:02
@legokidlogan legokidlogan marked this pull request as ready for review September 28, 2024 05:30
Copy link
Member

@brandonsturgeon brandonsturgeon left a comment

Choose a reason for hiding this comment

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

Looks okay. A few comments and improvements.

I think the Beep functionality should be moved to a method on the base grenade class, also.

Comment on lines 94 to 134
function ENT:BubbleStartTouch( ent )
if not ent:IsPlayer() then return end
if not ent:Alive() then return end

-- Check if we're allowed to affect the player (there might be a build/pvp system, etc)
local allowed = false

hook.Add( "EntityTakeDamage", "CFC_PvPWeapons_AntiGravityGrenade_CheckIfDamageIsAllowed", function()
allowed = true

return true
end, HOOK_LOW )

local dmgInfo = DamageInfo()
dmgInfo:SetAttacker( self:GetOwner() )
dmgInfo:SetInflictor( self )
dmgInfo:SetDamage( 100 )
dmgInfo:SetDamageType( ent:InVehicle() and DMG_VEHICLE or DMG_GENERIC )

ent:TakeDamageInfo( dmgInfo )
hook.Remove( "EntityTakeDamage", "CFC_PvPWeapons_AntiGravityGrenade_CheckIfDamageIsAllowed" )

if not allowed then return end

-- Apply the effect
ent:SetGravity( self.GravityMult )

if ent:IsOnGround() then
ent:SetVelocity( Vector( 0, 0, self.PushStrength ) )
end

ent._cfcPvPWeapons_AntiGravityGrenade = self

local rf = RecipientFilter()
rf:AddPlayer( ent )

sound.Play( "ambient/machines/machine1_hit2.wav", ent:EyePos(), 75, 120 )
util.ScreenShake( ent:EyePos(), 3, 5, 1.5, 500, true, ent )

return true
end
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is going to work like you think it will.

A much simpler and more reliable option:

  • Simply deal the damage here
  • Have a new PostEntityTakeDamage hook that checks for cfc_simple_ent_antigrav_grenade as the inflictor, and that took == true, and then run the second part of the effect

I believe the only difference will be that the bubbles will pop on non-pvpers, but I think that's fine

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't use PETD because I don't want to actually deal damage, merely check to see if it would otherwise pass damage checks. Note how it adds a temporary, low-priority hook which blocks the damage.

Also it does work, it's been tested live on the server for over a week straight.

Copy link
Member

Choose a reason for hiding this comment

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

I was saying you could deal 0 damage, and use PETD to verify that it actually passes all damage filters rather than toggling a hook in a Touch callback

Copy link
Member Author

Choose a reason for hiding this comment

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

But if I do that, it'll create a hitmarker, and potentially other strange behavior from having the damage event make it all the way to PETD.

Comment on lines +31 to +36
if not setModelScale then
local entMeta = FindMetaTable( "Entity" )

-- Get the original function, in case the server wraps it for anticrash purposes.
setModelScale = entMeta._SetModelScale or entMeta.SetModelScale
end
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use the function, wrapped or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

What? It is being used. This unwrap is needed here because of our own anticrash.

Copy link
Member

Choose a reason for hiding this comment

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

What does our anticrash wrap do that doesn't work here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It blocks scales above 3, and the bubbles go above that.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could exclude this on the anticrash side then? What if there are multiple wraps with different names and you don't actually get the unwrapped one you want

Copy link
Member Author

Choose a reason for hiding this comment

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

I could, but the only two ways I can think of aren't pretty. Either Making a hyper-specific name for the unwrapped function, or stuffing the wrapped function with a new argument into to force-ignore the limit, which wouldn't be accessible by e2 or sf due to them not accounting for more arguments.

Copy link
Member

Choose a reason for hiding this comment

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

What about just

if ent:GetClass() == "cfc_simple_ent_bubble" then return og( ... ) end

?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be painfully specific, and add an __index call and long string comparison to every SetModelScale() call. If we do things on the anticrash's end, it should be generalizable.

Comment on lines +138 to +140
-- Remove the bubble.
-- Without the extra 1-tick delay, this will sometime cause a MASSIVE lag spike due to the cosmetic bubbles being removed while SetModelScale's deltatime is still running.
-- No idea why that would happen, nor why it only happens with the cosmetic bubbles, but this fixes it.
Copy link
Member

Choose a reason for hiding this comment

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

Could you isolate and report this? Sounds like an easy bug report for rubat

Comment on lines +31 to +36
if not setModelScale then
local entMeta = FindMetaTable( "Entity" )

-- Get the original function, in case the server wraps it for anticrash purposes.
setModelScale = entMeta._SetModelScale or entMeta.SetModelScale
end
Copy link
Member

Choose a reason for hiding this comment

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

What about just

if ent:GetClass() == "cfc_simple_ent_bubble" then return og( ... ) end

?

@legokidlogan legokidlogan merged commit 1a9d454 into main Dec 6, 2024
1 check passed
@legokidlogan legokidlogan deleted the feature/grenades branch December 6, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants