MichaelAquilina / Some-2D-RPG

Tinkering with Game Development
MIT License
28 stars 5 forks source link

Astar Pathfinding #10

Closed behindcurtain3 closed 11 years ago

MichaelAquilina commented 11 years ago

This looks like a good implementation, however id like to be able to test it before i merge it in if you dont mind.

I took a look at your 'TheArena' repository which i saw was based on the Game Engine i am building, but i am having some trouble running it - the file boom.png cannot be found which prevents me from running the application. I tried removing it from the csproj file to see if it would work but i ran into more problems.

I also noticed you put a lot of work into more animation files and some other game objects, well done! :) If you dont mind, i will probably take some of the anim files to put into my repository for later use (unless you wish to do this yourself with a pull request).

Thanks for the help!

behindcurtain3 commented 11 years ago

Looks like I forgot to add the file, although it doesn't do anything yet. Go ahead an copy all the animations you like. I've been trying to import as many as I can, I don't see a need to do a pull request for them.

Regarding the AStar, I think the neighbor generation needs a little extra work. It currently only looks at itself when selecting neighbors and doesn't check if the neighbor is valid (Impassable or doesn't have Bottom set when it is a neighbor to the Top etc). I'm going to spend a little time to fix this.

MichaelAquilina commented 11 years ago

It seems you need to provide your version of GameEngine.dll for me to be able to run TheArena, because i am getting errors stating the PathFinding namespace doesnt exist. Could you provide me with a way of retrieving or building your forked version of the GameEngine project?

behindcurtain3 commented 11 years ago

You should just be able to clone my fork and then checkout the "astar" branch.

MichaelAquilina commented 11 years ago

Ok so i managed to take a look at the running implementation of your A Star algorithm in TheArena, and it looks good. There are a few times it crashed for me and there are some design points that i would like to change with it - but otherwise well done!

Once you feel you have finished everything, i will go ahead and merge in the pull request. I will probably perform some re factoring to ensure that the Pathfinding algorithm remains decoupled from the TiledMap, but otherwise i dont think i would need to change anything.

behindcurtain3 commented 11 years ago

Cool, I'm glad you checked it out. The only way I think it would crash right now is if you clicked outside the map (which is bad design on the way I was testing it, but not related to the actual pathfinding code).

I went ahead and refactored the code outside the TiledMap class and added some documentation. The one thing that is debatable is whether or not the GeneratePath should be static, For now, I removed the static part and you can call it by:

engine.Pathfinding.GeneratePath(start, target);
MichaelAquilina commented 11 years ago

Ill go ahead and merge it in once i take a look at it on your branch. Are there any optimizations being used currently?

behindcurtain3 commented 11 years ago

The only real optimization its doing now is building the valid neighbors for each node beforehand. Otherwise, its just a straight implementation of A*.