-
Notifications
You must be signed in to change notification settings - Fork 338
Add prop-break related functions #3300
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
base: master
Are you sure you want to change the base?
Conversation
Cvar and check if the ent is not npc should be made, will do. Regarding tildas - they're there to bring down these hooks in stack order, although I'm not sure if they're even alphabetically ordered. Regadring abuse - perhaps, but so far - couldn't make it abusable. Addons like contraption core work just fine. Maybe I should also restrit it to physical prop, so sents wouldn't be able to be mare invulnerable. Regarding returning true - again, I don't want to break other addons like contraption core, as, in theory, returning true will result in other hooks in the stack not running, thus removing ability for other addons to do whatever they want |
Return true is bad because it prevents other damage hooks from firing. We want to remove the damage, not block the event. Starfall's version of this function is limited to prop_physics class entities. Might be good to limit that here too. |
They aren't. It's random based on however lua iterates a string key'd table with 'pairs'. |
I think we should store the unbreakable status of an entity in its own table rather than in a separate one. |
Co-authored-by: Astralcircle <142503363+Astralcircle@users.noreply.github.com>
you shouldnt be running damage hooks when you're doing 0 damage |
Some things use the damage hook to tell if a prop gets hit tho, so they'd not detect hits on unbreakable things if you did that. It's kind of up to user interpretation though I guess. |
https://gmodwiki.com/GM:PostEntityTakeDamage exists for those cases |
Even tho it does, it does not mean that other addons use it. We don't want to break other addons, and there is no real benefit to returning true for us as well, only potential breakage of third-party addons. |
Try return true and see if the PostEntityTakeDamage works. If so then we should go with that. |
other addons receiving 0 damage is going to cause issues |
Yes, why change the damage if returning |
Yep, just tested. Returning true prevents Peek.2025-04-14.23-19.mp4 |
Starfall uses the built in engine damage filter, which might do the same thing. Maybe doesn't matter? Is there a reason to detect damage on unbreakable props? |
Well the non-breakable-by-default props still get both hooks correctly. I The return true tho is even worse, because some things may want just the |
If damage isn't being done it shouldn't be running damage hooks, this is unexpected behavior and will cause issues with other addons. |
Could maybe be useful for there to be a hook to decide if a entity can be made unbreakable but considering it's only prop_physics i suppose it's fine as is |
if not ValidAction(self, this, "break") then return end | ||
if this:GetClass() ~= "prop_physics" then return self:throw("This entity can not be made unbreakable!", nil) end | ||
|
||
this.wire_unbreakable = this:Health() > 0 and breakable == 0 and 1 or 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be this:Health() > 0 and breakable == 0 and true or nil
I already do this in my modified version of unbreakable tool and it causes absolutely no problems, because why should there be any problems? Why call another hook just to emphasize the lack of damage? |
Once #3300 (comment) and swapping out ScaleDamage(0) to returning true are addressed this pr is good to go |
@@ -714,6 +715,31 @@ e2function void entity:propBreak() | |||
this:Fire("break",1,0) | |||
end | |||
|
|||
hook.Add("EntityTakeDamage", "WireUnbreakable", function(ent, dmginfo) | |||
if ent.wire_unbreakable then dmginfo:ScaleDamage(0) end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ent.wire_unbreakable then dmginfo:ScaleDamage(0) end | |
if ent.wire_unbreakable then return true end |
if not ValidAction(self, this, "break") then return end | ||
if this:GetClass() ~= "prop_physics" then return self:throw("This entity can not be made unbreakable!", nil) end | ||
|
||
this.wire_unbreakable = this:Health() > 0 and breakable == 0 and 1 or 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.wire_unbreakable = this:Health() > 0 and breakable == 0 and 1 or 0 | |
this.wire_unbreakable = this:Health() > 0 and breakable == 0 and true or nil |
No description provided.