Feature/4.0/update core types #465
Conversation
piiertho
force-pushed
the
feature/4.0/update-reimplemented-core-types
branch
from
fecd58f to
499a0cb
Compare
| /** | ||
| * Returns true if the AABB is empty. | ||
| */ | ||
| fun hasSurface(): Boolean { |
There was a problem hiding this comment.
Comment is incorrect, it returns false if empty
There was a problem hiding this comment.
Former comment that I did not checked. Will change it.
| /** | ||
| * Returns true if the AABB is flat or empty. | ||
| */ | ||
| fun hasVolume(): Boolean { |
| @@ -535,6 +587,237 @@ class Color( | |||
| val r = (hex and 0xFFFF) / 65535.0 | |||
| return Color(r, g, b, a) | |||
| } | |||
|
|
|||
| private val namedColors = arrayOf( | |||
There was a problem hiding this comment.
Should add more consistency here. Those names already exist as variables, meaning we create them two times in two different ways (one with constructor and one with hex). Should be merged so we only create them one time.
| val Double.signbit: Boolean | ||
| get() = sign < 0 | ||
|
|
||
| fun cubicInterpolate( |
There was a problem hiding this comment.
Shouldn't that go in the MathFuncs.kt you created in that same PR ?
| require(size.x >= 0 && size.y >= 0 && size.z >= 0) { | ||
| "AABB size is negative, this is not supported. Use AABB.abs() to get an AABB with a positive size." | ||
| } |
There was a problem hiding this comment.
Is it a good idea to crash here? Shouldn't we log an error and return false instead?
I know godot "crashes" in such cases but don't we then have issues if such a call originated from the cpp side?
Same for other usages of require and the likes.
There was a problem hiding this comment.
I think we should better explain to jetbrains they have a vision regarding preprocessors that is not compatible with game industry.
Here I would expect a debug version of engine to have this require call and crash, to spot problems asap, and a release version without this call.
Anyway, this should not be modified in this PR IMO, as we have a lot of already existing require in core types.
| // Check if the amount of hex digits is valid. | ||
|
|
||
| // Check if the amount of hex digits is valid. |
| // Check if each hex digit is valid. | ||
|
|
||
| // Check if each hex digit is valid. |
| // Flip quaternions to shortest path if necessary. | ||
| // Flip quaternions to shortest path if necessary. |
| // Calc by Expmap in from_q space. | ||
| // Calc by Expmap in from_q space. |
| // Flip quaternions to shortest path if necessary. | ||
| // Flip quaternions to shortest path if necessary. |
| // Calc by Expmap in from_q space. | ||
| // Calc by Expmap in from_q space. |
| @@ -36,7 +36,7 @@ class StringName : NativeCoreType { | |||
| return TransferContext.readReturnValue(VariantType.STRING, false) as String | |||
| } | |||
|
|
|||
| //TODO/4.0: Implement | |||
| operator fun <T> invoke(stringOperation: String.() -> T): T = toString().stringOperation() | |||
There was a problem hiding this comment.
Not sure what I'm looking at here. What would a typical use case of this invoke operator look like?
There was a problem hiding this comment.
StringName has a LOOOOOOT of functions that would be cumbersome to reimplement, even as Bridge, and they happen to be the same as the regular Godot String. So instead of spending time implementing that, we decide to just add a utility function that can transform a StringName using the regular Kotlin String methods.
There was a problem hiding this comment.
hmm i see. but then i would a dd a documentation on the function with a link to this page and a hint that one can reimplement those functions with this operator, and probably also provide an example of such a reimplementation using this operator
…ons, in core types
This updates all core types for godot 4.0.