Skip to content

ItemShouldTakeDamage hook added #87

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.

Sign up for GitHub

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

Jump to bottom
Merged
TovarischPootis merged 1 commit into from
Nov 8, 2021
Merged

ItemShouldTakeDamage hook added #87

TovarischPootis merged 1 commit into from
Nov 8, 2021

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