View previous topic - View next topic |
Author |
Message |
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 2:27 am Post subject: Help with scrolling |
[quote] |
|
so here goes: I'm trying to get the map to scroll between tiles. I'm having a tough time because the map won't move and I'm getting quick exits out of the program every time I try to move in a direction
I loaded the tiles in from the .txt file correctly, I loaded them into SDL_Rects appropriately, and I'm sure I can get the currentTile correctly.
My problem alies with drawing. I have to deal with offsets, player input, etc. etc. and It's not seeming to work.
Any help would be appreciated: you can find the demo here
Basically look in the "Engine.cpp, Map.cpp/.h, and Player.cpp" files. Map has all the drawing functions in it, with the ability to calculate where to draw them. Engine handles the main game loop, updating player, recieving player's position, etc., and Player is just the general player class.
Ninkazu's code can be found here, and it's about half-way down the page.
I've tried to use that to my advantage, but it's no use. The pseudo is good, but not eligible to work in my coding. I'm almost there though.
Thanks a bunch,
Jason
|
|
Back to top |
|
|
Rainer Deyke Demon Hunter
Joined: 05 Jun 2002 Posts: 672
|
Posted: Wed Feb 15, 2006 3:29 am Post subject: |
[quote] |
|
Looking at your Map::Draw function, I can't point to a single mistake since it's hard to figure out how this is supposed to work in the first place. So instead, I'll just give you a function that works.
First off, there are several exception safety bugs in your code. Never use raw pointers to handle owned resources unless you are writing your own smart pointer class. Turn m_mapIndexes and m_tiles into a std::vectors. Turn m_graphics into a boost::scoped_ptr (or std::auto_ptr if you don't have boost installed yet).
Now for the actual code. I can't make sense of your camera handling code, but that's a separate issue from actually drawing the map, so I am leaving it outside the function. Just pass in the actual pixel offsets at which you want to draw the map. This function safely handles the case where the map only covers part of the screen.
Code: |
void Map::Draw(int x_offset, int y_offset)
{
int min_x = std::max(0, x_offset / m_tileSize);
int max_x = std::min(m_mapWidthTileCount,
(x_offset + m_screenWidth) / m_tileSize);
int min_y = std::max(0, y_offset / m_tileSize);
int max_y = std::min(m_mapHeightTileCount,
(y_offset + m_screenHeight) / m_tileSize);
for (int y = min_y; y < max_y; ++y) {
for (int x = min_x; x < max_x; ++x) {
int tile = m_mapIndexes[x * m_mapHeightTileCount + y]
SDL_Rect source = m_tiles[tile];
SDL_Rect dest = {
x * m_tileSize - x_offset,
y * m_tileSize - y_offset,
tileSize,
tileSize,
};
m_graphics->DrawBitmap(m_bitmap, source, dest);
}
}
}
|
|
|
Back to top |
|
|
LeoDraco Demon Hunter
Joined: 24 Jun 2003 Posts: 584 Location: Riverside, South Cali
|
Posted: Wed Feb 15, 2006 3:29 am Post subject: |
[quote] |
|
I don't savvy SDL, and I frankly am not terribly interested in debugging your exact problem here, but I would definitely offer some tips to simplify and improve your source code. You know: to cut the redundant fat, leaving a lean, muscular code base, ready to be imbued with extensibility.
For your benefit, should you be interested:
- Division is a (relatively) slow process, and the results of most operations are not guaranteed to be cached, ever. So, in your for loops in Map::Draw()? Either flip the direction of your loop (so that the expensive division is only performed once per loop (which slightly improves the inner loops, but only slightly: instead of executing that division for each loop iteration, you only calculate it once per outer loop iteration.) or pre-calculate those values before any loop that utilizes them, er, utilizes them.
- Enumerations are really poor constructs for representing type information, especially within an OOPL. (Seriously: they should be deprecated; the handful of legitimate uses are painfully uncommon.) Map::HandleState() and Map::HandleWalkState() are perfect examples of this: to imbue extra functionality upon your type enumerations, you have to perform that massive switch construct, which is both inefficient in most cases, as well as being horribly unmaintainable. The better solution is to utilize proper object hierarchies to represent those types, which, essentially, would introduce a new class for each enumerated constant; each class would be a sub-class of a new class, the name of which is the same as the old enum name. Obviously, you would want to remove the enumerations. If you are considered about the time it takes to perform comparisons, you could make each of those objects singleton flyweights, in which case comparison would simply between pointers.
- Initialization lists are the preferred method for constructing objects; typically, the resulting code is slightly cleaner, slightly more intuitive, and slightly faster.
- Sprinkled about the source are references to widths and heights, or references to two-dimensional coordinates; however, in neither case are these data intrinsically tied to each other, which presents a logistics problem: if nothing else, it leads to moderate code-bloat. A far better solution would be to introduce a construct to represent these data: something like a "Point2D" class, which would store two data (say, an x and a y coordinate). All operations on points could be conducted within that class --- adding to the encapsulation of the project. This also introduces semantic value to your source code that currently does not exist: you can unequivocally state that the object on which you are operating is a two-dimensional point, and that fact is reflected within your souce code. Now, "Point2D" is rather static, and can lead to rather annoying curiosities, should you ever decide to reuse code in a 3D project. An alternative would be to use templating:
Code: | template< int Size >
class CartesianPoint
{
...
public:
double Coordinate( int Which ) const throw ( out_of_bounds )
{
if Which >= Size then throw out_of_bounds();
return _Coordinate[ Which ];
}
...
private:
double _Coordinate[Size];
...
}
...
CartesianPoint< 2 > TwoDPoint;
CartesianPoint< 3 > ThreeDPoint;
|
Or something like that; note that should be a form of template metaprogramming, which should be optimizable by the compiler; however, I have not tested that code, so do not take my word for it. Also, as C++ currently does not support useful initializer list constructs for classes, this might not be the easiest method for representing generic points...
Input::HandleInput()? Crazy! At the very least, you should be considering all key-press events as a single event trigger --- that is, a single keyboard poll should be sufficient to test against all keys. A simple switch --- which is an abhorent method to handle this, but still slightly better than what you have --- should suffice. Also, why are you performing the same action three straight times in a row? (I'm talking about the SDL_PollEvent call.) If you are worried about catching the poll event, you can do something like loop until the method returns a value, sleeping a short period in between, or, quite likely the better mechanism, automatically return if the method does not return a valid event. (Seriously: GLUT has this slightly better, as everything in it is handled by registered callbacks, which are called whenever it fires an event; while I do not care so much for VB.NET (or for .NET at all), its MVC setup is at least moderately intelligent for this type of thing.)
Should this code be intentionally unoptimized --- say, this is stuff that has been vastly simplified to ease the grokking --- I apologize for any, ah, sagely advice offered. _________________ "...LeoDraco is a pompus git..." -- Mandrake
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 3:42 am Post subject: |
[quote] |
|
Lol, the Draw function was just a frustration of variables and calls I kept changing and moving around. Of course it sucks.
As for my actual programming organization....
it's hard. I've had no formal training. I don't know what's the "best" way to do something. I'm self taught, and I'm doing things how I figure them out.
I don't know anything about optimization, or flyweights. I'm glad that I do now though, and I thank you for that.
And the input was to fix the miss-handled events. The game would lag whenever I'd move the mouse and hit different keys (because it had to wait another loop before it could take the message). When I tried clicking and moving the mouse and pressing keys, it got worse.
So I did 3 polls lol. Again, I could have had a for loop to handle all messages, but what fun is that?
So how should I go about organizing this? I'd gladly start over now before continuing on and getting worse. don't forget, I've had no formal training (I'm 17) and I'm doing this logically. Is there any steps I can take or things to read on what to do with my program and make it more organized?
And thanks for the code deyke :)
Thanks,
Jason
|
|
Back to top |
|
|
Rainer Deyke Demon Hunter
Joined: 05 Jun 2002 Posts: 672
|
Posted: Wed Feb 15, 2006 4:03 am Post subject: |
[quote] |
|
My advice would be not to worry too much about organization at this point, and especially don't worry too much about LeoDraco's suggestions. Division is plenty fast enough - if you can do perspective texture mapping (which requires one division per pixel) in software on a 486, you won't even notice a couple of divisions per tile. Enumerations aren't always the best tool for the job, but they're syntactically lightweight and they work. Especially stay away from the CartesianPoint template - that's overabstraction, which can lead to just as much code bloat as underabstraction. (And never ever use a floating point variable when an integer will do.) (And no, this is not template metaprogramming. It's just generic programming, aka programming with templates. Metaprogramming implies writing actual code which is executed at compile time.)
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 4:05 am Post subject: |
[quote] |
|
By the way, that code doesn't make it scroll.
It draws the tiles nicely,but it won't move. If I'm incrementing/decrementing the offsets in my switch statement for direction, shouldn't it be moving it?
Jason
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 4:07 am Post subject: |
[quote] |
|
Here's my actual code if interested: It's all my updated variables and such.
Code: | void Map::Draw()
{
int min_x = max(0, m_offsetX % m_tileSize);
int max_x = min(m_mapWidthTileCount,(m_offsetX + m_screenWidth) / m_tileSize);
int min_y = max(0, m_offsetY % m_tileSize);
int max_y = min(m_mapHeightTileCount,(m_offsetY + m_screenHeight) / m_tileSize);
for (int x = min_x; x < max_x; ++x)
{
for (int y = min_y; y < max_y + 1; ++y)
{
int tile = m_mapIndexes[y * m_mapHeightTileCount + x];
m_source = m_tiles[tile];
m_destination.x = x * m_tileSize - m_offsetX;
m_destination.y = y * m_tileSize - m_offsetY;
m_destination.w = m_tileSize;
m_destination.h = m_tileSize,
m_graphics->DrawBitmap(m_bitmap, m_source, m_destination);
}
}
}
void Map::HandleState()
{
switch (m_currentState)
{
case Walk:
HandleWalkState();
break;
default:
HandlePauseState();
break;
}
Draw();
}
void Map::HandlePauseState()
{
}
void Map::HandleWalkState()
{
switch (m_currentDirection)
{
case South:
m_amountToMove = m_movementSpeed / m_currentFps;
m_cameraY += m_amountToMove;
m_offsetY += m_amountToMove;
if (m_cameraY > m_mapHeight - m_screenHeight)
m_cameraY = m_mapHeight - m_screenHeight;
break;
case East:
m_amountToMove = m_movementSpeed / m_currentFps;
m_cameraX += m_amountToMove;
m_offsetX += m_amountToMove;
if (m_cameraX > m_mapWidth - m_screenWidth)
m_cameraX = m_mapWidth - m_screenWidth;
break;
case North:
m_amountToMove = m_movementSpeed / m_currentFps;
m_cameraY -= m_amountToMove;
m_offsetY -= m_amountToMove;
if (m_cameraY < 0)
m_cameraY = 0;
break;
case West:
m_amountToMove = m_movementSpeed / m_currentFps;
m_cameraX -= m_amountToMove;
m_offsetX -= m_amountToMove;
if (m_cameraX < 0)
m_cameraX = 0;
break;
default:
break;
}
CheckBoundaries();
}
void Map::CheckBoundaries()
{
if (m_offsetY >= m_tileSize || m_offsetY <= 0)
{
m_offsetY = 0;
}
if (m_offsetX >= m_tileSize || m_offsetX <= 0)
{
m_offsetX = 0;
}
} |
|
|
Back to top |
|
|
LeoDraco Demon Hunter
Joined: 24 Jun 2003 Posts: 584 Location: Riverside, South Cali
|
Posted: Wed Feb 15, 2006 4:12 am Post subject: |
[quote] |
|
Gardon wrote: | it's hard. I've had no formal training. I don't know what's the "best" way to do something. I'm self taught, and I'm doing things how I figure them out. |
That is alright, but that is also why it is important to pick up tricks when they are offered. Er, from multiple people, in case the instructor turns out to be a talentless git.
Quote: | So how should I go about organizing this? I'd gladly start over now before continuing on and getting worse. don't forget, I've had no formal training (I'm 17) and I'm doing this logically. Is there any steps I can take or things to read on what to do with my program and make it more organized? |
You should definitely read up on Design Patterns, which are generally the best way to think about designing code. (Caveat: they are not perfect; in fact, in the hands of a complete idiot, they can be downright onerous. However, that is a vacuous excuse: nearly anything in the hands of an idiot can be dangerous.) Also, as you are utilizing objects which sort of resemble event handlers, you will probably want to read up on the Observer pattern and MVC in particular.
As for structure? Well, that sort of borders on the limits of style, something that every coder must develop. However, if nothing else, take this into consideration: if something seems overly complicated, you are either overthinking it --- in which case, you should redesign --- or you are not utilizing a suitable level of abstraction --- in which case, you should refactor. _________________ "...LeoDraco is a pompus git..." -- Mandrake
|
|
Back to top |
|
|
Rainer Deyke Demon Hunter
Joined: 05 Jun 2002 Posts: 672
|
Posted: Wed Feb 15, 2006 4:41 am Post subject: |
[quote] |
|
1. Use the function that I have written, exactly as I have written it. Your new function has introduced several new bugs. (The '%' should be '/'. The 'y + 1' should be just 'y'.) Then use another function as a wrapper around it, like this:
Code: |
void Map::Draw()
{
Draw(m_offsetX, m_offsetY);
}
|
2. Your CheckBoundaries function is throwing away all movement. Either ditch it entirely, or replace it with something like this:
Code: |
int clip(int value, int low, int high)
{
return std::max(low, std::min(high, value));
}
void Map::CheckBoundaries()
{
m_offsetX = clip(m_cameraX - m_screenWidth / 2, 0, m_tileSize * m_mapWidthTileCount - m_screenWidth);
m_offsetY = clip(m_cameraY - m_screenWidth / 2, 0, m_tileSize * m_mapWidthTileCount - m_screenWidth);
} |
Better yet, ditch the m_offset variables entirely and calculate the offsets from the camera during rendering:
Code: |
void Map::Draw()
{
int x_offset = clip(m_cameraX - m_screenWidth / 2, 0, m_tileSize * m_mapWidthTileCount - m_screenWidth);
int y_offset = clip(m_cameraY - m_screenWidth / 2, 0, m_tileSize * m_mapWidthTileCount - m_screenWidth);
Draw(x_offset, y_offset);
}
|
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 4:57 am Post subject: |
[quote] |
|
Yes, thank you, but it's not smooth scrolling. And even so, it only lets me scroll over one tile and then the opposite button returns it.
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 4:59 am Post subject: |
[quote] |
|
it's almost as if I'm scrolling to the edge of the map when I click the buttone
|
|
Back to top |
|
|
Rainer Deyke Demon Hunter
Joined: 05 Jun 2002 Posts: 672
|
Posted: Wed Feb 15, 2006 5:06 am Post subject: |
[quote] |
|
Looking at your code again, it seems that m_cameraX and m_cameraY already contain the clipped offset. So just use this (and ditch the m_offset variables entirely, if you haven't done so already):
Code: | void Map::Draw()
{
Draw(m_cameraX, m_cameraY);
} |
As an experiment, you might want to turn off all camera clipping. This would allow you to scroll past the edges of the map.
Also, there was a bug in my code. Instead of these lines:
Code: | int max_x = std::min(m_mapWidthTileCount,
(x_offset + m_screenWidth) / m_tileSize);
int max_y = std::min(m_mapHeightTileCount,
(y_offset + m_screenHeight) / m_tileSize); |
Use these lines:
Code: | int max_x = std::min(m_mapWidthTileCount,
(x_offset + m_screenWidth + m_tileSize - 1) / m_tileSize);
int max_y = std::min(m_mapHeightTileCount,
(y_offset + m_screenHeight + m_tileSize - 1) / m_tileSize); |
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 5:11 am Post subject: |
[quote] |
|
still not working. It shifts me to the end of the map and stops.
Whatever man, you tried, and I thank you for it. However, I get very frustrated over things like this, which gives me more ambition to do it right the first time.
I'll try re-writing it tomorrow and make it work.
Thanks,
Jason
|
|
Back to top |
|
|
RuneLancer Mage
Joined: 17 Jun 2005 Posts: 441
|
Posted: Wed Feb 15, 2006 5:29 am Post subject: |
[quote] |
|
Eh, sometimes a break can help. There's some psychological quirk that goes something like, when you can't pull something off, the more you try it the harder you'll fail until you take a break.
Just move your offset along with the player sprite, but draw the sprite in the middle of the screen. (Ie, move the map instead of the sprite ;)) It's simple and a little primitive (you'll see why once you hit the edges of the map) but it's real easy to do and you can work something better from there. _________________ Endless Saga
An OpenGL RPG in the making. Now with new hosting!
|
|
Back to top |
|
|
Gardon Scholar
Joined: 05 Feb 2006 Posts: 157
|
Posted: Wed Feb 15, 2006 5:37 am Post subject: |
[quote] |
|
I think you're right. This shouldn't be that hard.
I do that a lot with projects. I'll code something for 3 hours, and not get it, and get so freaken frustrated I just junk the project and start over.
Lol, that same project might take me a half hour to do, simply because I've cleared my mind and started over.
I just wish I could do everything right the first time and not waste so much on failing and starting over.
Jason (as you can tell I'm still so engrossed into it I'm checking the forum every 5 minutes rather than going to bed)
|
|
Back to top |
|
|
|
Page 1 of 3 |
All times are GMT Goto page 1, 2, 3 Next
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
|