Skip to content

ItemShouldTakeDamage hook added#87

Merged
TovarischPootis merged 1 commit intoNutScript:1.2.3-wipfrom
Rorkh:ItemShouldTakeDamage
Nov 8, 2021
Merged

ItemShouldTakeDamage hook added#87
TovarischPootis merged 1 commit intoNutScript:1.2.3-wipfrom
Rorkh:ItemShouldTakeDamage

Conversation

@Rorkh
Copy link
Contributor

@Rorkh Rorkh commented Nov 8, 2021

I think it is essential for such hook to be since item entity have health and there no way to make all items or just a part really invulnerable for all or some types of damage without hacks like ent:setHealth(9999999999).
It's named like PlayerShouldTakeDamage and works similarly too.

@TovarischPootis
Copy link
Contributor

TovarischPootis commented Nov 8, 2021

My only qualm is the naming convention
NS uses camelCase, not PascalCase
Though that's a nitpick that I'll let others discuss

Otherwise, seems like a viable addition

@Rorkh
Copy link
Contributor Author

Rorkh commented Nov 8, 2021 •
edited
Loading

My only qualm is the naming convention NS uses camelCase, not PascalCase Though that's a nitpick that I'll let others discuss

Otherwise, seems like a viable addition

Really camelCase for hooks? Search by hook.Run via GitHub search bar inside repository. And...
https://nutscript.miraheze.org/wiki/Category:Hooks

@TovarischPootis
Copy link
Contributor

TovarischPootis commented Nov 8, 2021

My only qualm is the naming convention NS uses camelCase, not PascalCase Though that's a nitpick that I'll let others discuss
Otherwise, seems like a viable addition

Really camelCase for hooks? Search by hook.Run via GitHub search bar inside repository. And... https://nutscript.miraheze.org/wiki/Category:Hooks

Touché

@TovarischPootis TovarischPootis merged commit e3c4b0c into NutScript:1.2.3-wip Nov 8, 2021
@FlorianLeChat
Copy link
Contributor

FlorianLeChat commented Nov 8, 2021

I don't think adding this hook is a good idea. Simply because it is a duplicate of the EntityTakeDamage hook. With this addition, both hooks will strictly fire under nearly the same conditions. Honestly, I don't think triggering similar hooks just to save an if statement is a good thing for efficiency.

@Rorkh
Copy link
Contributor Author

Rorkh commented Nov 9, 2021

Oh. Really. I forgot about it for some reason. Revert if you want.

TovarischPootis added a commit that referenced this pull request Nov 9, 2021
@TovarischPootis
Revert "ItemShouldTakeDamage hook added (#87)"
589535f 
This reverts commit e3c4b0c.
@TovarischPootis TovarischPootis mentioned this pull request Nov 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TovarischPootis TovarischPootis TovarischPootis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Rorkh @TovarischPootis @FlorianLeChat