faronbracy / roguesharp Goto Github PK

Steps to reproduce:

1. Create a new map of any size
2. Call the map.GetBorderCellsInSquare(x: 0, y: 0, distance: 1) method with the given parameters

Expected results:
Since we are in the corner of the map and choose a distance of 1, the border cells returned in the list should be only the following three cells: {1,0},{1,1},{0,1}

Actual results:
Besides the three cells stated above, the center cell from which the border selection began is also in the list. {0,0}

The .editorconfig for this solution is pretty bare on rules. Consider adding the rest of the rules supported by Visual Studio as well as Resharper.

Once the rules are in place, make sure that the code conforms to them and there are not any errors.

The DepthFirstPaths class has a PathTo method that returns a list of vertices from source to destination
https://github.com/FaronBracy/RogueSharp/blob/master/RogueSharp/PathFinder.cs#L100
The source vertex is not included in the list.

This behavior is different from the PathFinder class's ShortestPath method which does include the source cell.
https://github.com/FaronBracy/RogueSharp/blob/master/RogueSharp/PathFinder.cs#L100

The behavior should be consistent across all methods that return paths

Currently the GetCell method of Map returns a concrete Cell class. While you can inherit Map and implement your own features, there's no way to stuff your implementation of the ICell interface into your implementation of Map as the GetCell method always returns a concrete type of Cell.

To be able to have your own ICell implementations maybe Map needs a generic or something (similar to IMapCreationStrategy) or GetCell should be virtual with GetCell where T is ICell.

Previously FxCop was built into Visual Studio and RogueSharp had a ruleset that it used to do static code analysis.

Static analysis is now delivered via nuget packages:
https://github.com/dotnet/roslyn-analyzers#recommended-version-of-analyzer-packages

Get this setup for RogueSharp with a sensible set of rules.

If the Map class had a custom indexer it would allow us to call Map[X,Y] to get a cell at an X,Y coordinate.

I have received feedback that this would be slightly nicer than calling Map.GetCell( X, Y ) as is available now.

Edit: I guess I should say "convert from struct to class". My editor said it was "sealed" but I checked the code and it's a struct. Maybe you have compelling perf reasons to keep it a struct.

I was very surprised that I couldn't subclass the Point class. I know I can use composition instead. I'm using an ECS pattern where components implement an IComponent interface (for serialization). Some of the components are just points with a different name. LocationComponent and DestinationComponent for example. It would be a lot cleaner for them to inherit point so I can do things like subtract them.

var delta = location - destination
entity.AddComponent(delta);

V.S.

var delta = location.point - destination.point
var deltaComponent = new DeltaComponent(){point = delta};
entity.AddComponent(deltaComponent)

The DijkstraShortestPath class has a Check method that is currently only used by unit tests.
https://github.com/FaronBracy/RogueSharp/blob/master/RogueSharp/Algorithms/DijkstraShortestPath.cs#L153

It was originally intended to be a private method and called from the bottom of the constructor.

Investigate if this method is useful or not. If it's not get rid of it. If it is useful consider making it private and make the class use it where appropriate.

https://en.wikipedia.org/wiki/A*_search_algorithm

This should perform better than the current Dijkstra algorithm.

Any plans on adding a third dimension with pathing information?

• First of all having the option to add layers allows apply the logics to up/down too.
• Secondly with a proper pathing information and algorythm one could implement either flying things (free vertical movement) or grounded pathing (only transition between layers on special transition cells).

一个400*400的地图寻路一次消耗1-2M 内存。同时多个怪物寻路导致内存不足,大概在1小时后。需要调用GC.Collect(); 才能回收内存。

The map generation in RogueSharp is very generic and not suitable for creating maps for real games. They were meant to be more of an example that could be expanded upon.

Consider including something similar to https://github.com/odedw/karcero in RogueSharp

Currently the existing behavior of FoV creates a diagonal pattern rather than a circular pattern FoV. I was going to try to override the behavior of FoV, but there's no way to do so because of how tightly bound and hidden the logic is in Map & Cell. I saw a similar issue posted on the old Bitbucket site. Also saw your post here: https://roguesharp.wordpress.com/2017/04/25/roguesharp-4-0-pre-release/ but that won't allow us to overwrite the behavior of Cell.IsInFov.

DijkstraShortestPath.PathTo method returns an IEnumerable and could yield return DirectedEdge.
https://github.com/FaronBracy/RogueSharp/blob/master/RogueSharp/Algorithms/DijkstraShortestPath.cs#L130

Currently it does not, yet many other methods in the solution that return IEnumerable do.
https://github.com/FaronBracy/RogueSharp/blob/master/RogueSharp/Map.cs#L293

Investigate which approach is better and be consistent within the project.

RogueSharp currently multi-targets .NET Standard 1.0 and .NET Framework 4.0.

Consider bumping this up to .NET Standard 2.0.

Pros

• Additional features available to make development more pleasant
• Unity now supports .NET Standard 2.0

Cons

• Platforms that don't support .NET Standard 2.0 but that did support 1.0 will no longer be able to use the latest version of RogueSharp

Example

• The {color} door is made of {material}.
• This corridor smells like {smell}. It has {feature} along the walls.

https://en.wikipedia.org/wiki/Mad_Libs

Im subclassing the Map class and using the CaveMapCreationStrategy, during the point of CellularAutomataBigAreaAlgorithm the map is cloned and cast as the type specified in the IMapCreationStrategy interface. however this clone returns null and throws a null reference exception.

I think the clone method should create a new T() rather than a new Map()

For example, this will throw the error:

public class CavesMap : Map
{
}

void Test()
{
CaveMapCreationStrategy<CavesMap> strategy = new CaveMapCreationStrategy<CavesMap>(width, height, 40, 4, 3);

CavesMap map = strategy.CreateMap();
}

Seems there's a lot of changes to v5.0 that I took a look at but it broke most of my code and I can't find alternative ways to figure out the way to move forward, so I have to go back to the last release until either I write my own functions to replace the old missing ones or figure out how to migration v4 RogueSharp to v5. Would like to see some documentation on how to do this.

Specifically here's a few things that break in v5 that I don't see a clear path to fix:

• ComputeFov is no longer in the Map class. I know there's a FieldOfView class but it doesn't seem to be used anywhere. I think you're supposed to use the FieldOfView class on the fly and pass the Map in and use it's ComputeFov from it?
• Cells no longer have a value for if they're explored or not (IsExplored) so do we have to keep track of this ourselves in our own Cell implementations?

Thanks

Instead of calling Map.SetCellProperties( x, y, isTransparent, isWalkable, isExplored) have 3 simple methods for setting these properties.

Map.SetIsTransparent(x,y,true);
Map.SetIsWalkable(x,y,true);
Map.SetIsExplored(x,y,true);