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

Add missing extension functions + prop window fix #82

Merged
merged 4 commits into from Jun 10, 2018

Conversation

Projects
None yet
4 participants
@Jampi0n
Copy link
Contributor

Jampi0n commented Jun 1, 2018

Added missing getCurrentValue functions for functions that already had getDefaultValue: PropWindow, TurnSpeed, AcquireRange

Added functions with angle as parameter/return value for prop window functions.
Fixed a problem with getDefaultPropWindow:
The native returns the value in degrees, so it has to be converted to radians first. Usually all natives use radians.

Typo: AcquireRange

Add missing extension functions + prop window fix
Added missing getCurrentValue functions for functions that already had getDefaultValue: PropWindow, TurnSpeed, AcquireRange

Added functions with angle as parameter/return value for prop window functions.
Fixed a problem with getDefaultPropWindow:
The native returns the value in degrees, so it has to be converted to radians first. Usually all natives use radians.

Typo: AcquireRange
@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jun 1, 2018

if the native returns it, an implicit conversion is even worse imo.
The getters should also return angle tuples.

@Jampi0n

This comment has been minimized.

Copy link
Contributor Author

Jampi0n commented Jun 1, 2018

The problem is, that GetUnitDefaultPropWindow uses degrees and GetUnitPropWindow uses radians, which makes it inconsistent.
There are getters that use angle, so they will be safe. I can change the functions that use real to return the same as the native and add hotdoc, so the user knows whether it is degrees or radians.
Similar to how it was done with facing angle, I used getPropWindow and getPropWindowAngle.

Jampi0n added some commits Jun 1, 2018

Hotdoc + removed implicit conversion
Added hotdoc to clarify whether radians or degrees are used for the prop window functions.
Reverted unit.getDefaultPropWindow() to return the value in degrees like the native function does.
Add functions to set max hp/mana that adjust the current value to kee…
…p ratio

The new blizzard natives do not adjust the current value. Abilities and upgrades adjust the current value to keep the same ratio between current and max value.
This adds extensions functions, so hp and mana can be changed without changing the ratio between current and max value.
@Trokkin

This comment has been minimized.

Copy link
Contributor

Trokkin commented Jun 3, 2018

+1 for angle tuples, as far as we have them as standart. I think there shouldn't be a single function across the entire stdlib that takes or returns angle as a real instead of the tuple for consistency reasons.

@Jampi0n

This comment has been minimized.

Copy link
Contributor Author

Jampi0n commented Jun 3, 2018

Ok I will remove the functions that return real then.
As I already said, I did it like this, because it's the same with getFacing and getFacingAngle.
Also regarding the latest commit. The name for the function is quite long, if you know a better name, let me know.

@Trokkin

This comment has been minimized.

Copy link
Contributor

Trokkin commented Jun 3, 2018

Hm, maybe you could do an overload for old function with a flag that decides whether to save ratio or not.

@crojewsk

This comment has been minimized.

Copy link
Contributor

crojewsk commented Jun 3, 2018

public function unit.setMaxHp(int hp, boolean keepRatio)
public function unit.setMaxMana(int mana, boolean keepRatio)

That's sound about right.

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jun 3, 2018

Yes, I agree. We want angle safety everywhere so the user never has to worry about mixing up radians/degrees again.

Angle safety + overload setMaxHP/Mana
Added deprecated annotation for angle functions using reals.

Overload setMaxHP/Mana instead of using a seperate function. A boolean decides whether ratio is kept. The boolean will be constant in most cases, so it will be inlined in the end.
@Jampi0n

This comment has been minimized.

Copy link
Contributor Author

Jampi0n commented Jun 4, 2018

Only uses angle type for functions that use angles. Two old functions that still used reals have been annotated with deprecated.
As suggested setMaxHp/Mana was overloaded with a boolean keepRatio. Wurst's optimizer will remove the condition, as the boolean will be constant in most cases.

@Frotty

This comment has been minimized.

Copy link
Member

Frotty commented Jun 10, 2018

Yea looks good now, thanks 🍺

@Frotty Frotty merged commit 1efd8a3 into wurstscript:master Jun 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Jampi0n Jampi0n deleted the Jampi0n:Units-Package branch Jun 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment