Jump to content

  • Log In with Google      Sign In   
  • Create Account

We need your help!

We need 1 more developer from Canada and 12 more from Australia to help us complete a research survey.

Support our site by taking a quick sponsored survey and win a chance at a $50 Amazon gift card. Click here to get started!


Is this bad?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
11 replies to this topic

#1 Crusable   Members   -  Reputation: 594

Like
0Likes
Like

Posted 06 May 2013 - 07:27 PM

Hello, I am working on a project in sfml and I am making a very basic tile map editor. I am working on the tile selection and this is how I got the selected tile to snap to a 64* 64 tile:

 

 
m_sfSelectedTilePos.x = (float)(((int)m_sfEvent.mouseButton.x / 64) * 64);
m_sfSelectedTilePos.y = (float)(((int)m_sfEvent.mouseButton.y / 64) * 64); 


Sponsor:

#2 ultramailman   Prime Members   -  Reputation: 1719

Like
0Likes
Like

Posted 06 May 2013 - 08:02 PM

I don't think it is bad, but I think it is better to work with integers. Assumming the position of a tile is the top left corner, then a tile at index (1, 2) should have the position (1 * 64, 2 * 64) = (64, 128). So the float position can be calculated only when absolutely needed.

So then the index of a tile would be calculated by
m_sfSelectedTileIndex.x = m_sfEvent.mouseButton.x / 64;
m_sfSelectedTileIndex.y = m_sfEvent.mouseButton.y / 64;

Edited by ultramailman, 06 May 2013 - 08:06 PM.


#3 Crusable   Members   -  Reputation: 594

Like
0Likes
Like

Posted 06 May 2013 - 08:27 PM

When you mean work with ints do you mean make all my floats into ints. Most SFML's classes I am using uses sf::Vector2f's.



#4 frob   Moderators   -  Reputation: 31426

Like
3Likes
Like

Posted 06 May 2013 - 09:05 PM

Two maxims: Programmers should never repeat themselves, and magic numbers are bad.

 

 

Create a function to snap to a tile.  Perhaps use the signature Vector2F SnapToTile(Vector2F source) { }.

 

Second, rather than using the constant 64, instead use a static const int defined in a header file somewhere.

 

 

Your version will always snap to the nearest border, and perhaps someday you would rather the function snap to the nearest tile boundary.


Check out my book, Game Development with Unity, aimed at beginners who want to build fun games fast.

Also check out my personal website at bryanwagstaff.com, where I occasionally write about assorted stuff.


#5 ultramailman   Prime Members   -  Reputation: 1719

Like
0Likes
Like

Posted 06 May 2013 - 09:33 PM

When you mean work with ints do you mean make all my floats into ints. Most SFML's classes I am using uses sf::Vector2f's.

Not at all, the position can still be float. Simply multiply the index by 64, and cast to a float.

#6 ByteTroll   Crossbones+   -  Reputation: 2324

Like
0Likes
Like

Posted 06 May 2013 - 09:41 PM

Two maxims: Programmers should never repeat themselves, and magic numbers are bad.

 

 

Create a function to snap to a tile.  Perhaps use the signature Vector2F SnapToTile(Vector2F source) { }.

 

Second, rather than using the constant 64, instead use a static const int defined in a header file somewhere.

 

 

Your version will always snap to the nearest border, and perhaps someday you would rather the function snap to the nearest tile boundary.

 

Your way of doing things is not bad, but Frob's way will probably work better in the long run.  Quickly adding to Frob's answer, I would poll the values from somewhere.  Most editors have a drop down menu that lets you select snapping values.


"The code you write when you learn a new language is shit.
You either already know that and you are wise, or you don’t realize it for many years and you are an idiot. Either way, your learning code is objectively shit." - L. Spiro

"This is called programming. The art of typing shit into an editor/IDE is not programming, it's basically data entry. The part that makes a programmer a programmer is their problem solving skills." - Serapth


#7 Matias Goldberg   Crossbones+   -  Reputation: 5599

Like
2Likes
Like

Posted 06 May 2013 - 10:44 PM

I'd recommend using floor() rather than casting to int back and forth. The casting can be several times slower (if performance is an issue)

 

Note that floor( -3.3 ) = -4; while (int)-3.3 = -3; but floor's behavior is usually how you want snapping to work if negative numbers are possible.



#8 Crusable   Members   -  Reputation: 594

Like
0Likes
Like

Posted 06 May 2013 - 11:10 PM

Two maxims: Programmers should never repeat themselves...

What did you mean by this?



#9 DaleyPaley   Members   -  Reputation: 150

Like
0Likes
Like

Posted 07 May 2013 - 02:17 AM

By the way, you can use floor() instead of converting to ints:

 

m_sfSelectedTilePos.x = floor(m_sfEvent.mouseButton.x / 64) * 64;
 

Edit: oops, just saw that Matias already said it. Sorry!


Edited by DaleyPaley, 07 May 2013 - 02:18 AM.


#10 Álvaro   Crossbones+   -  Reputation: 16563

Like
1Likes
Like

Posted 07 May 2013 - 05:34 AM


Two maxims: Programmers should never repeat themselves...

What did you mean by this?


The code in the first post contains two lines of code that are almost identical. Chances are you can give the operation a good name and put it in a function.

I also recommend writing operations on vectors or points instead of having high-level code do everything component by component.
float snap_to_multiple(float x, float size) {
  return std::floor(x / size) * size;
}

Point snap_to_grid(Point p, float tile_size) {
  Point result;
  result.x = snap_to_multiple(p.x, tile_size);
  result.y = snap_to_multiple(p.y, tile_size);
  
  return result;
}

// ...

m_sfSelectedTilePos = snap_to_grid(m_sfEvent.mouseButton, 64.0f);


Edited by Álvaro, 07 May 2013 - 05:35 AM.


#11 irreversible   Crossbones+   -  Reputation: 1748

Like
0Likes
Like

Posted 07 May 2013 - 06:30 AM

A simple solution to force casting to snap the value is to use a half-unit offset:

 

iX = (int)(fX + 0.5f);

 

or to support both positive and negative values:

 

iX =  fX > 0 ? (int)(fX + 0.5f) : (int)(fX - 0.5f);

 

Note: casting to int truncates towards zero.

 

Edit: I was too rash - better solutions have been posted above since you're not dealing with unit intervals.



#12 DevFred   Members   -  Reputation: 840

Like
0Likes
Like

Posted 11 May 2013 - 08:25 AM

If you want to round an int down to a multiple of 64, you can use bitmasking:

 

x = x & ~63;

 

Note that this only works for powers of two.






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS