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

Base Attributes #2664

Merged
merged 43 commits into from
Jul 18, 2020
Merged

Conversation

Wealthyturtle
Copy link
Member

@Wealthyturtle Wealthyturtle commented Nov 29, 2019

Description

This PR is an improvement of #2656 that adds a new attributetype type and %entities%'s %attributetype% value expression, that allows for the direct modification of entities' attributes, such as attack speed attribute, max health attribute, etc. while also allowing for storage of attributetypes in variables.

Please help me review :)


Target Minecraft Versions: Any
Requirements: None
Related Issues: None

@Wealthyturtle Wealthyturtle added the feature Pull request adding a new feature. label Nov 29, 2019
Copy link
Member

@FranKusmiruk FranKusmiruk left a comment

Choose a reason for hiding this comment

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

The reason we have asked for a type is to be able to use the different enum constants as english phrases. Having an expression which uses a string is pretty much the same thing.

Look at how the Cat.Type or similar classes handle this behaviour.

@Wealthyturtle
Copy link
Member Author

Ohhh sorry... Now I see what you mean... These few commits should fix that.

@Wealthyturtle
Copy link
Member Author

By the way, is it alright for me to define all the attributes present in 1.14 in english.lang even if 2 of them did not exist in 1.9 (generic_armor_toughness & generic_flying_speed)?

@FranKusmiruk
Copy link
Member

Yes, they're just ignored in older versions.

@Wealthyturtle
Copy link
Member Author

Added a documentation notice that the movement speed attribute cannot be reliably used for players, and for that purpose, users should use the speed expression instead.

@bensku
Copy link
Member

bensku commented Dec 27, 2019

Could this be automatically tested, or does it require player interaction?

@Wealthyturtle
Copy link
Member Author

@bensku We could probably do something like setting an entity's attribute, waiting a tick, and then comparing if it's the same as before.

@bensku
Copy link
Member

bensku commented Dec 28, 2019

Even waiting a tick should not be necessary. Besides, delays are currently not supported in tests.

@FranKusmiruk FranKusmiruk modified the milestones: 2.6, 2.5 Jul 7, 2020
@FranKusmiruk
Copy link
Member

Is this ready for merge?

@Wealthyturtle Wealthyturtle changed the title Attributes Base Attributes Jul 17, 2020
@Wealthyturtle
Copy link
Member Author

Should be alright to merge. I can do a future PR to add more functionality onto this expression.

@FranKusmiruk FranKusmiruk dismissed stale reviews from bensku and Pikachu920 July 18, 2020 00:52

Changes were made

@FranKusmiruk FranKusmiruk merged commit 351bbb2 into SkriptLang:master Jul 18, 2020
@Wealthyturtle Wealthyturtle deleted the feature/attributes2 branch July 18, 2020 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants