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

Add map() function, for mapping values from a range to another #10195

Closed
hunar1997 opened this issue Aug 9, 2017 · 37 comments
Closed

Add map() function, for mapping values from a range to another #10195

hunar1997 opened this issue Aug 9, 2017 · 37 comments

Comments

@hunar1997
Copy link

I used Processing and it contains a very useful function map()
it works like this: var result = map(v, a,b, x,y) you pass a value v which is between a and b and the function will return a number between x and y

ex: var r = map(60, 20,100, 200, 240) this will return 220
or: var g = map(0.43, 0,1, 0,255)

From here i saw that the actual java code used is:
static public final float map(float value, float istart, float istop, float ostart, float ostop) {
return ostart + (ostop - ostart) * ((value - istart) / (istop - istart));
}

I really love to add it myself but i never worked in other's codes and i don't know how and where to put it so please somebody add it .. its very usefull

@ghost
Copy link

ghost commented Aug 9, 2017

You can do this in GDScript. Also map is a very vague, if not misused (which typically means this), term for this.

@vnen
Copy link
Member

vnen commented Aug 9, 2017

This seems an oddly specific function with an awful name (as @tagcup said, map usually means applying a function to all elements in a list). It seems to me the same as lerp but with the ability to select the range instead of using only 0-1. In fact, your second example (var g = map(0.43, 0,1, 0,255)) is exactly what lerp does.

@groud
Copy link
Member

groud commented Aug 9, 2017

Agree with vnen for the naming part. But i'm not even sure it's really needed, as we allready have lerp.
Anyway this could be called somehow like rlerp, (for range lerp) ?

Could be marked as Junior job I guess

@ghost
Copy link

ghost commented Aug 9, 2017

But why does this have to be in the core? You can implement such specific functions in two lines in gdscript.

@hunar1997
Copy link
Author

hunar1997 commented Aug 9, 2017

I agree that the name map() is wrong, i like rlerp() .. but it's very annoying to implement it every time in gdscript .. it's clearly more complicated than Vector2.length() so its like saying "implement Vector2.length() in your gdscript", there are alot of other functions that are very easy to implement in gdscript so why exclude this one? :/

@ghost
Copy link

ghost commented Aug 9, 2017

"Why exclude this one" is the wrong question, to wrong person.
Instead, you need to give a compelling reason to why this particular function should be in the core.

Everybody has that sort of utility functions. If we add all of those to the core, it will be a heap of incoherent utility box.

FYI, you can reuse functions defined in other files in GDScript.

@ghost
Copy link

ghost commented Aug 9, 2017

And if you need performance, you can use NativeScript for such functions now.

@groud
Copy link
Member

groud commented Aug 9, 2017

Well, this is a tool math function. I perfectly understand why it may be in the math API.
Usually we refer either to the C or the python API for those function. Can you find an equivalent there ?

@ghost
Copy link

ghost commented Aug 9, 2017

Why do think being mathematical justifies adding it to the core? I can literally give you thousands of useful mathematical functions (just open Mathematica's help, or check out Julia's libraries).

@hunar1997
Copy link
Author

hunar1997 commented Aug 9, 2017

It's very useful in changing a number from one range to another, i used it before in other places for changing world to pixel coordinates and vice versa (very useful in creating mandelbrot), i don't have any example for Godot in my head right now :/, but how bad is it to include something that is not very useful in core :|

@groud
Copy link
Member

groud commented Aug 9, 2017

Ok, so I can ask you the opposite question. Why not adding it to the core ? This is not a corner case function and it is useful, it seems, at least to Processing users.

We are not talking about some complex math function, this is just a math tool to interpolate from one range to another. It's easy to implement a faces many possible usage.

I still don't say should be added, but I understand that the question may be asked: This function is simple enough to serve in a lot of situations but it is also complex enough not to be in the core math API. The question is not simple.

@ghost
Copy link

ghost commented Aug 9, 2017

I'm not saying it's not useful for you. You need to give convincing argument as to why this is one-liner, which you can trivially implement on your own, has to be in the core for everyone.

FFT, for example, which isn't even trivial, is useful in various types games for example, but it's not there because it's not for relevant for most, and they can simply use fftw if they need it.

@ghost
Copy link

ghost commented Aug 9, 2017

@groud

Why not adding it to the core ?

I answered this exact question above. It doesn't work like that. You don't come up with random stuff and ask "ok explain to me, why not". You have to explain why it has to be in the core.

We are not talking about some complex math function, this is just a math tool to interpolate from one range to another.

Exactly, which he can implement on his own easily.

Anyway, this isn't going anywhere. I don't see any compelling reasons for adding this to the core, so I wouldn't add this.

@ghost
Copy link

ghost commented Aug 9, 2017

And my understanding is mathematical functions in the core support the needs of the engine itself, which is totally different from being a union of all utility functions various users came up for their own needs.

@groud
Copy link
Member

groud commented Aug 9, 2017

This is not a random function. It's used in Processing's API and is useful to OP.
Find me a good argument explaining me why lerpshould be in the core and not this rlerp ?
They basically do the same either from 0 to 1 or from A to B.

Saying no to API additions by default is absurd, else useful function like abs() would not have been added because its easy to code it your own way?

@ghost
Copy link

ghost commented Aug 9, 2017

Please properly read the two replies I wrote above right above your post.
I'm done with this otherwise.

@groud
Copy link
Member

groud commented Aug 9, 2017

And my understanding is mathematical functions in the core support the needs of the engine itself, which is totally different from being a union of all utility functions various users came up for their own needs.

Ok, that's where we don't agree :)
For me some functions that are useful to everyone, even not used in the engine code, should be available to use in GDscript via the math API.

@ghost
Copy link

ghost commented Aug 9, 2017

It's not useful to me.

@hunar1997
Copy link
Author

This was the first time i contribute in anything :/ It wasn't as great as i expected, it wasn't bad also .. Thanks anyways , I'm done too

@groud
Copy link
Member

groud commented Aug 9, 2017

@hunar1997 No worries, this is not happening every time. API changes are always tricky to introduce ^^
@tagcup it is not useful to you, but I think it would be to me. The point is to find out if it is useful to most users or if it is just making the math API too heavy. :) And, please don't be agressive, we all try to understand the point of view of each other.

@ghost
Copy link

ghost commented Aug 9, 2017

it is not useful to you, but I think it would be to me.

Yes, that's exactly the point I was trying to make. You're asking about adding an edge case stuff to the core (core means core of the engine). Surely they're useful to some. If you think this could be useful to other people, be my guest and add it on Assert Library. But please don't extrapolate it to everyone.

You don't see me asking "why not" for tensors, matrix exponents, integral transforms, general ode and pde solvers, higher dimensional matrices, complex matrices, nonabelian algebras, nonassociative algebras like octonions, linear algebra on binary or exotic rings, error correction codes, non-linear optimization algorithms, functionals, signal processing etc. etc. While useful and mathematical, none of these need to be in the core because the engine doesn't need them.

I apologize and I really don't want to be aggressive but it's quite irritating when you persistently get asked for the exact same thing you just explained.

@groud
Copy link
Member

groud commented Aug 9, 2017

But this is not the same level of math: we're talking here about 3 lines of code simplified in a one-liner. As with abs(), sign()... We're not talking here about a complex and corner-case treatment.

Also for me it is not an edge-case. I can come up with few examples where it could be used:

  • mapping a screen mouse position to a grid
  • mapping the x position of a node to a score (like with flappy bird ^^)
  • mapping an anchor value to its screen position (would have been useful to me in Control node enhancements #9889 I think :) )

@Zireael07
Copy link
Contributor

I can think of a couple uses for a map() function, be it in 2D or 3D. For instance, mapping non-binary input (joystick or some such, not keyboard which is always 0 or 1) to angles or velocities.

@ghost
Copy link

ghost commented Aug 9, 2017

Again, I'm not discussing this anymore. I'm recapping what I said for the umpteen time.

  • If you need it, you can add it to your project.
  • If it's a one-liner, all the better, you can add it yourself easily.
  • If you think it may be useful to others too, be my guest, put it on Asset Library.
  • Of course it's useful to some people. Otherwise, it wouldn't have existed in the first place. But that's not good enough for putting it into the core. Core (which is located under the directory core/) caters to the engine itself.
  • If something isn't already in the core, it's a good indication that engine in its current state doesn't need it. When people add stuff to the core, it's because something new in the engine needs it, or because something really needs to be fixed/improved.
  • All those mathematical stuff I mentioned above are very useful for many many things in game physics and games (actually, special cases of some of those are used internally in almost all engines). They still don't have a place in the core.
  • PRs in open-source projects don't work like that. You don't show up saying "here's my PR, either accept it or give me a satisfactory explaination why not!". You have to give compelling reasons why it should be added into the project and not somewhere else.

@Zireael07
Copy link
Contributor

@tagcup: Most game engines (or at least the ones that pretend to be universal, not niche specialized things like roguelike engines or shooter engines or whatnots) have such a function in core. I don't understand your resistance to this when we already have lerp in core?

@ghost
Copy link

ghost commented Aug 9, 2017

If you actually read my previous post, you'll discover that the answer is there.

@Zireael07
Copy link
Contributor

I still don't understand what the reasoning behind including this but not that in core is. Eg. the recent work on gradiented rounded styleboxes seems to be ok.

@bojidar-bg
Copy link
Contributor

bojidar-bg commented Aug 10, 2017

I recently needed this function in order to remap something (which I incorrectly call gravity_movement), to the range of 0..1. The result was a boring 151-char line (not counting tabs), of which I can hardly see half while coding:

var normalized_gravity_movement = (gravity_movement - gravity_movement_treshold_low) / (gravity_movement_treshold_high - gravity_movement_treshold_low) # 151-char
# Or, if we remove prefixes:
var normalized = (value - range_low) / (range_high - range_low) # 63-char
# Or, with the proposed function:
var normalized_gravity_movement = remap(gravity_movement, gravity_movement_treshold_low, gravity_movement_treshold_high, 0, 1) # 126-char
var normalized = remap(value, range_low, range_high, 0, 1) # 59-char

The result is less repetition (I don't have to repeat the gravity_movement_treshold_low variable), as well as more understandable code ("aha! He is remapping the value", as opposed to "Uh-oh, complex math ahead!").

That's why I'd support such a function (expressiveness and ease of understanding).


As to @tagcup's points:

  • If you need it, you can add it to your project.

There is no reason we can't ship one. This is just an utility method, similar to lerp, which is pretty much on the same level of complexity.

  • If it's a one-liner, all the better, you can add it yourself easily.

Here is how ternary ifs could be implemented in GDScript before they were PRed:

var value = [if_false, if_true][int(condition)]
# Or:
var value = {true: if_true, false: if_false}[condition]

Complex and messy oneliners, right?
But, still, they were implemented in "core". Even though I still haven't used those more than once or twice, I guess there are many users who find uses for them every day.

  • If you think it may be useful to others too, be my guest, put it on Asset Library.

I think it would be useful to many. That's why I want it on by default, in core (see my next point).

  • Of course it's useful to some people. Otherwise, it wouldn't have existed in the first place. But that's not good enough for putting it into the core. Core (which is located under the directory core/) caters to the engine itself.

By "core", we don't mean it has to be in core/ - it might be in modules/gdscript/ as well; to the end-user "core" stands for everything which is there without additional plugins.

  • If something isn't already in the core, it's a good indication that engine in its current state doesn't need it. When people add stuff to the core, it's because something new in the engine needs it, or because something really needs to be fixed/improved.

Take for example #9782, the YXZ rotations issue.
It wasn't implemented because of many requests for it, or being a roadblock to interesting projects.
It was implemented as an utility addition to be used when making games, even when it is more proper to use axis-angle rotations.

  • All those mathematical stuff I mentioned above are very useful for many many things in game physics and games (actually, special cases of some of those are used internally in almost all engines). They still don't have a place in the core.

They don't figure in core, as they are used in just a few places. They are a bit complex to most users, which is another reason not to put them there. And since you rarely need most of them when making a game (unless you need neural networks), there isn't enough reason to expose them.
Though, I have to admit, tensors and matrixes (for LA) would be a huge boost for some, so feel free to open an issue 😃 .

  • PRs in open-source projects don't work like that. You don't show up saying "here's my PR, either accept it or give me a satisfactory explaination why not!". You have to give compelling reasons why it should be added into the project and not somewhere else.

There are my reasons. And I don't see why we shouldn't add a feature that is going to be somewhat useful to users, if it won't bloat the download by more than 1KB.

@ghost
Copy link

ghost commented Aug 10, 2017

#9782 is added for something the editor needs, which is a part of the engine.

@ghost
Copy link

ghost commented Aug 10, 2017

And the PR for it just replaces the implementation of Euler angles from XYZ to YXZ, it actually doesn't even expose any new function to users, so I don't know why you're talking about it.

@groud
Copy link
Member

groud commented Aug 10, 2017

I think the problem is that you consider that a function should be used in the editor code to be in the core API. This is just not true, nothing like that have been ever decided.

Godot is first a game engine before being an editor, so its core API is first meant to run video games before running the editor. Thus, if a function is needed in several games, is not a corner case, and is asked by enough people, we should add it to the core API even if it is not used in the editor code.

@ghost
Copy link

ghost commented Aug 10, 2017

Anyway, do whatever you think is right. I said my piece: I'm against this, for reasons stated above. You don't need to convince me, because it's not going to be my decision.

@ghost
Copy link

ghost commented Aug 10, 2017

You can't just keep adding functions to the core (and not asset library or somewhere else) whenever somebody shows up with a random function and shows some in-game use case. Weighing factors will be different for simple functions and complicated one, but you still need to draw a line somewhere. Otherwise, you'll end up with a incoherent heap of random stuff.

@groud
Copy link
Member

groud commented Aug 10, 2017

I perfectly agree with that, but allways,the question is still where to draw the line. I perfectly understand why you would think that lerp or range_lerp should not be in the math API, they are indeed somehow specific to some use cases.

But, I am sorry, but you cannot just tell someone that proposes a change to the API that his proposal is just a "random function" that would make the API "incoherent" or bloated. This is contemptuous, or just not nice.

Anyway, let's just stop this here. #10225 will probably be discussed in the next PR monday (on IRC) and submitted to a vote. :) We'll see.

@ghost
Copy link

ghost commented Aug 10, 2017

Surely I can tell that a particular function shouldn't be in the core if I don't think it shouldn't be in the core. I live in a free country. Jeez, please don't make this personal. Especially I gave multiple lengthy, non-personal, merit-based explanations. Just keep it at technical level and read what I write properly before making ad hominem accusations (like I never said he's like like this or that, that was a general argument not targeted toward an individual).

@akien-mga
Copy link
Member

Alright, let's stop the fight here gentlemen, thanks.

Whether the feature is wanted or not can be discussed during our Monday meeting for PR review, as @groud mentioned above.

@jhusak
Copy link

jhusak commented Jan 2, 2024

Well, this function named "remap" was added in godot 4.

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

No branches or pull requests

8 participants