Skip to content

Conversation

@nova0x0
Copy link
Contributor

@nova0x0 nova0x0 commented Sep 3, 2025

The command now no longer works if the target of the command is in pvp mode and the caller is not an admin.

ULib.tsayError( caller, ulx.getExclusive( ply, caller ), true )
elseif not ply:Alive() then
ULib.tsayError( caller, ply:Nick() .. " is dead and cannot be propified!", true )
elseif ( ply.IsInPvp and ply:IsInPvp() ) and not caller:IsAdmin() then
Copy link
Member

Choose a reason for hiding this comment

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

This is going to prevent moderators from using the command on pvpers, im not sure thats desired.

Copy link
Member

Choose a reason for hiding this comment

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

if you want to go ahead with your idea of an spropify command then maybe it would be better to drop the check here and add it to that command instead.

Copy link
Member

@plally plally left a comment

Choose a reason for hiding this comment

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

This looks good, great job.

Comment on lines 200 to 202
for _, ply in pairs( targets ) do
if ply.ragdoll then ply.ragdoll:CPPISetOwner( ply ) end
end
Copy link
Member

Choose a reason for hiding this comment

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

If this is meant to resolve the issue of explosive models allowing builders to kill pvpers, thus opening the command for non-staff usage, this should be moved to the base command.

I'll also mention that this would allow players to manipulate their prop self with chips, which could lead to some wacky stuff but shouldn't cause any problems. Would be fun to see, honestly.

Copy link
Member

Choose a reason for hiding this comment

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

i think its just to allow builders to do funny stuff with the command.
Would probably need to have the owner set to the caller instead if its applied to the normal command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is meant to resolve the issue of explosive models allowing builders to kill pvpers, thus opening the command for non-staff usage, this should be moved to the base command.

I believe explosive models can already kill pvpers, regardless of who they are owned by. This was mainly to allow prop manipulation with chips, and also grant prop protection buddies the ability to physgun/toolgun the prop.

Would probably need to have the owner set to the caller instead if its applied to the normal command.

I have moved this call to propifyPlayer and changed the owner to be the caller rather than the target.

Copy link
Member

Choose a reason for hiding this comment

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

Explosion damage from props will be attributed to (and thus check pvp status of) whoever dealt damage to the prop last, and fallback to :CPPIGetOwner() if no damage was dealt.

Testing in p2p, this all works fine, preventing build abuse... except for when the prop explodes by impacting the pvper's body. In such a case, the prop takes physics damage from the pvper, causing the explosion attacker to be the pvper, and thus letting the damage go unblocked and counting as a suicide in the killfeed.

Granted, this can already be abused by builders without needing to use this command, so it would be best solved by having cfc_pvp block DMG_BLAST damage when the inflictor is a builder-owned, non-player entity. No further changes needed in this pr.

- These calls are redundant, as prop deletion is already handled by propify.
- DisallowDeleting also enforces a toolgun whitelist on the affected prop,
which is undesired behavior.
@nova0x0 nova0x0 requested a review from legokidlogan September 5, 2025 15:50
@plally plally merged commit 7851ed8 into CFC-Servers:master Sep 6, 2025
2 checks passed
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