Closed Chris3606 closed 5 years ago
Much of the time, the 'resistance' parameter is going to be used simply as true/false. Someone using the library can think of this case in two ways:
if mapCell.blocksFov ---> set fovMap[i,j] = 1.0
or
if mapCell.isFovable ---> set fovMap[i,j] = 0.0
I personally think the first way is slightly more intuitive since I associate '1' with 'true' and '0' with 'false'. That said, I'm actually using the second in my project because I much prefer the 'isTrait' style of boolean names over the 'doesSomething' style. If the FOV numeric convention is reversed it becomes
if mapCell.blocksFov ---> set fovMap[i,j] = 0.0
or
if mapCell.isFovable ---> set fovMap[i,j] = 1.0
When I see these two options, they both fit better in my mind. When I read "blocksFov --> 0.0", I'm thinking in terms of light amount. When I read "isFovable --> 1.0" I'm thinking in terms of '1'='true'.
If I imagine a more sophisticated multi-dimensional vision system, I think the reverse convention is going to be better as well. Consider a dark--->light + cold--->hot system where some entities are invisible to normal light but can be seen on infrared scopes - and other entities can be seen visually but can 'run cold'. Something in between those is an entity with camouflage that can adjust their heat to ambient temperatures (so they can hide in both dimensions of vision).
In this system, you are constantly going to be measuring things like
lightFovMap[i,j] = creature.visibility or mapCell[i,j].lightLevel
heatFovMap[i,j] = Math.Abs(creature.temperature - mapCell[i,j].temperature)
And these semantic operations only work in the context of the reverse of the current GoRogue convention. Without them, if I was implementing such a vision system (which I plan to do in the near future), I would need a lot of '1.0 - bigCalculation' to have my semantics match the logic of the FOV convention. This has the potential to make code a lot harder to read and introduce a lot of hard-to-spot bugs.
You should hear from as many as possible before changing a convention since it's a really big 'permanent' decision for such a library.
In addition, I wanted to say that this library is amazing. There is so much utility (small but important stuff that one would otherwise spend huge amounts of time writing and rewriting and bug-fixing), and it's all very compact. The only thing it "lacks" is a more extensive wiki as I'm sure there are many cool tricks and features that I haven't gleaned yet.
When I see these two options, they both fit better in my mind.
Mine as well, the more I think about it, and I do concur that it enables the application of math to more complex systems in an easier manner.
Furthermore, I believe it would be more consistent with the "walkability map" produced by map generation, where walkable tiles have a value of true, and non walkable tiles have a value of false. Commonly, at least in early stages of development, "walkable" also means "transparent". In this context, the proposed change also translates more directly; we have
// 1.0 associated with "true" in walkability map
if mapCell.isWalkable ---> set fovMap[i, j] = 1.0
You should hear from as many as possible before changing a convention
I definitely agree here as well, particularly because this change would break existing code in a somewhat subtle manner, without generating a compiler error. I plan to leave this question here, in hopes of receiving more feedback, before making the change.
In addition, I wanted to say that this library is amazing.... The only thing it "lacks" is a more extensive wiki
Thank you! Feedback is always much appreciated, and I'm glad to see it well-received, and to see it get utilized. As for the wiki, as you can see the framework for a more extensive wiki is in place on the GitHub Wiki page, it's just that most of the appropriate pages have not been completed. This is actually next on the agenda, after I finish releasing 1.3.0 (which to be honest might be better served as a major version release because it practically rewrites RNG and makes a number of other tweaks that improve the library but to an extent break backwards compatibility). I also plan on potentially creating more of a game example tutorial down the road, however I have been waiting until I get a few more portions implemented (including goal maps, which will be coming soon with a bit of luck).
This change will likely end up in the 1.4 GoRogue release, coming soon.
Going to postpone this, as it would require some significant changes to calculations in the algorithms, and while they should be easily manageable, I don't want to delay the release of other features via this testing.
Because of backwards compabibility issue, we'll revisit this in 2.0 (which shouldn't be too far down the line, with some luck)
This issue is actually less relevant when #103 is implemented, as it will change the resistance scaled definition. This is the only way to maintain compatibility with current use cases, as well as allow for use cases like the ones above (albeit with some 1 - math). Effectively, it is the only way to increase flexibility of the algorithm while not completely irreparably breaking existing use cases.
Currently, SenseMap/FOV take as a parameter an IMapOf, where the values range from 0.0 to 1.0.
A value of 0.0 represents 0 resistance to light (a fully transparent cell), and 1.0 represents full resistance to light (a fully opaque cell). Would this make more sense the other way around, where 1.0 is full transparency (100% of light gets through), and 0.0 is fully opaque (0% of light gets through)?