GraphicsConfig spritePath() hidden behavior about chessplusplus HOT 7 CLOSED

::chesspp::config::GraphicsConfig is a deriving class from ::chesspp::config::Configuration. We could just add a memember function to Configuration to get the raw NestedValue with any json path you like. However I don't think we should change spritePath as it is specifically for the chesspp namespace - we would use a GraphicsConfig in a different namespace otherwise.

from chessplusplus.

My argument is that it's not a namespace. It's a path inside the json file. If you're looking at the json file, it doesn't make sense to me to have to explicitly leave out "chesspp" when creating paths. I see no reason why there couldnt be another top level node beside chesspp.

That is, spritePath("chesspp", "board", "image") is incorrect in the current code and spritePath("board", "image") is correct.

That initially makes sense but it's another thing to think about when you're looking at the json file. "Oh, I know the path is chesspp/board/image, but I don't need to specify the first part."

If I'm the only one that has this issue then I'll gladly forget about it.

-Stephen Hall

On Dec 24, 2013, at 4:18 PM, "LB--" [email protected] wrote:

::chesspp::config::GraphicsConfig is a deriving class from ::chesspp::config::Configuration. We could just add a memember function to Configuration to get the raw NestedValue with any json path you like. However I don't think we should change spritePath as it is specifically for the chesspp namespace - we would use a GraphicsConfig in a different namespace otherwise.

—
Reply to this email directly or view it on GitHub.

from chessplusplus.

I can see both sides here. It did throw me off and seem like unexpected
behavior, though.

On Tue, Dec 24, 2013 at 3:58 PM, Thumperrr [email protected] wrote:

My argument is that it's not a namespace. It's a path inside the json
file. If you're looking at the json file, it doesn't make sense to me to
have to explicitly leave out "chesspp" when creating paths. I see no reason
why there couldnt be another top level node beside chesspp.

That is, spritePath("chesspp", "board", "image") is incorrect in the
current code and spritePath("board", "image") is correct.

That initially makes sense but it's another thing to think about when
you're looking at the json file. "Oh, I know the path is
chesspp/board/image, but I don't need to specify the first part."

If I'm the only one that has this issue then I'll gladly forget about it.

-Stephen Hall

On Dec 24, 2013, at 4:18 PM, "LB--" [email protected] wrote:

::chesspp::config::GraphicsConfig is a deriving class from
::chesspp::config::Configuration. We could just add a memember function to
Configuration to get the raw NestedValue with any json path you like.
However I don't think we should change spritePath as it is specifically for
the chesspp namespace - we would use a GraphicsConfig in a different
namespace otherwise.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/67#issuecomment-31186064
.

from chessplusplus.

@Thumperrr the thing is, the class in question is mean to be a convenience. In another namespace would would create a similar class with the same convenience. However now that I think of it it would be better if the json file itself were in a chesspp directory than to have chesspp mentioned in the json.

from chessplusplus.

The only problem I have with that is I don't like mixing C++ terminology and concepts in with json terminology and concepts, which are two different beings of their own. When i look at the json file I don't consider "chesspp" a namespace. I just consider it a root node that's the beginning of a path that gets to the node i'm looking for. If that's the case then I should include it in my search path.
I know it's meant to be a convenience to have "chesspp" specified for you. And I would be considering it as such if I didn't just have that little fiasco with ResidentBiscuit trying to figure out what the hell went wrong. Then, looking back on it, I started to feel like we should have to specify "chesspp" should the chesspp node exist at all.

from chessplusplus.

I think we should get rid of the chesspp from the json altogether and put the json in a chesspp subfolder.

from chessplusplus.

I can agree to that. I'll do that quickly and then open a PR.

from chessplusplus.