cpluspluscom / ChessPlusPlus

cplusplus.com Community Project: Modular Chess with support for more types of pieces than traditional Chess
https://cpluspluscom.GitHub.IO/ChessPlusPlus/
36 stars 26 forks source link

Design and Flexibility #2

Closed computerquip closed 10 years ago

computerquip commented 11 years ago

So, I've been thinking of something to come up with in design. To begin, let's get some terminology out of the way:

Board: A grid consisting of valid locations for a piece to move.

Piece: An entity that contains logic on how to move and other entities like itself.

Game: Generally consisting of a board and a set amount of pieces (although not always the case), it handles global logic such as when the game ends, when a piece changes (such as when a pawn reaches the end of the board in a default game), and so on.

Please note that these definitions may need to be extended later since it's difficult coming up with defined terminology to fit the humongous amount of chess variations.

Moving on, there are some things that we know for sure:

So using, this... we get pretty much nothing. :8ball: So, my idea so far consists of implementing the logic of the game using generic virtual types. In example:

class Location {
    int8_t x;
    int8_t y;
    //int8_t z; //Perhaps a 3D variation?
}

class Piece {
    virtual bool move(short location) = 0; //Returns true on success, fail on failure..
    virtual bool attack(short location) = 0;

    std::string name;
    Location location;
};

class Board{
    virtual void place(Piece* piece, Location location) = 0; //This should throw exception on fail.
    virtual int move(Location curLoc, Location newLoc) = 0; //No exception, returns error or 0 on success.
};

namespace default {

//We have no reason to make this class virtual that I can see 
class Game {
    Game(Board* board) { /* Place pieces on the board. */ }

    /* 
    Returns number of moves made. No exception.
    This also checks for "rules" such as a check or checkmate and whether the game has a next turn.
    */
    int next();
}
}

Obvious flaws are whether the piece should move or the board should move the piece or the game should move the piece. I like the idea of virtual interfaces but I'm not so sure about the interface I'm introducing... this is where I fall short most of the time anyways. Any ideas would be lovely.

Also, I'm starting to think there's no reason to seperate "Game" and "Board" since game needs to know the dimensions of the board anyways.

naraku9333 commented 11 years ago

I was also thinking a server could maybe store a game state and if a client gets temporarily disconnected it could resume the game. But I do agree a central server model would be pretty complicated to really consider now but could be something to implement later.

LukeLeber commented 11 years ago

I'll draft up some higher level networking stuff over the next few days. Handling disconnections is very feasible.

jaredready commented 11 years ago

Looks as if LucidChart did not save some changes to the diagrams. I'll work on that tomorrow at some point. Luckily I saved a JPEG copy. And looks like Thumper got his saved here.

Thumperrr commented 11 years ago

Yeah, apparently LucidChart didn't save, despite my telling it to. Luckily, I downloaded a .pdf before I exited. I added a docs folder to the repo to hold anything of the sort.

Lowest0ne commented 11 years ago

I spent some time working on the logic of the game via console. I have used inheritance for the pieces. The direction of the pawn is calculated upon construction (if it's black it goes down). I would like to see the ability to have black on the bottom, especially if I was playing as black, but perhaps that would be in the future. Making black on bottom could possibly just be a difference in how the positions are interpreted.

Valid moves is seeming a little tricky. Right now I'm making a "list" of valid moves, so if the move one picks is in the list, the move must be valid. I do think better than calculating on the fly. The tricky part seems like it is going to be checking whether the move would put the king in check. I'm thinking maybe a piece have a trajectory, the valid moves being a sub set. Maybe something like "that piece can't move because the king is in the white piece's trajectory, and it happens to be blocking it".

Maybe I'm making it more complicated than it has to be. The other way I can think of would be "faking" the move, then checking every piece of the other color to see if it could hit the king.

Should a piece know why it can't move? Should a piece know the pieces it's pinning?

jaredready commented 11 years ago

LowestOne,

Have you posted your code anywhere? I've actually been working on the same, I'm curious what route you've taken with this.

Lowest0ne commented 11 years ago

I haven't. I don't know how pleased I am right now with it. I had trouble saturday night getting sf::renderwindow to work. I know I've got it working on other projects, but I wont have time to try again until Wed.

Lowest0ne commented 11 years ago

My fork now has the start of the game logic. You could actually play a game of chess. There is no check, checkmate, en-passe, or castling. You can take pieces though :)

I've done some things to the SFML files, but I'm pretty sure they aren't the latest. I've obvisously changed the events and render functions. I've also changed the way the looping works.

The game sleeps unless it gets a message, and sleeps between handling messages. I don't think noticeable as far as gameplay, but certainly for trying to do other things with your computer.

Thumperrr commented 11 years ago

Wow, you've done some work. And yeah, the application framework you've got is a bit behind, but that can be easily changed. I'll look over it some more when I get home from class tonight.

naraku9333 commented 11 years ago

I'm impressed, you've gotten a lot done. Moving, capturing, showing valid moves all work. Alt Wow

Lowest0ne commented 11 years ago

Thanks.

Just found this site, pretty neat stuff. http://chessprogramming.wikispaces.com

LB-- commented 11 years ago

Currently it seems that @Lowest0ne is performing massive refactoring. Comparing to his fork, you wouldn't even know they were the same codebase.

@Lowest0ne: What was wrong with the current code base and why are you drastically changing it? Looking at some of your code it seems to be far more complex than the original working code and much less modular. Could you explain your design to us?

EDIT: Also, it doesn't properly conform to the styleguide (but we weren't finished changing the existing code to conform either).

Lowest0ne commented 11 years ago

The logic of the game is now completely different, yes.

The way it interacts with the rest of the program is not that different, so I'll talk about that first. The logic of graphics has been taken out and placed in the appstategame.hpp. I feel the purpose of graphics is to draw things, not to know about the things that it is drawing.

That's the most of it. Something I did that I am open to changing (or at least, feel I need to say I'm open about) is the fact that AppStateGame knows what piece is selected and is "current". I did this because I felt that these are things that are results of the user clicking the mouse and didn't belong in a logic based class. Maybe a bad feeling.

About the major change in the logic.

There is no more inheritance. I feel it makes it more complicated than it has to be, particularly when it comes to checking for moves that make check and blocking moves that might cause check.

A piece is really a uint32_t with various masks for it's members, easily changeable if wanted.

A piece's valid moves is no longer a list (so called), but a std:bitset<64>. The list implementation was noticeably slow when it came to dealing with check. We had something like this:

-for_each( piece of other color ) --for_each( move in that piece ) ---for_each( move in this king) ----if ( king's move is enemy's move) -----remove move from king's move

And that doesn't even work with pawns.

Now we have this, which still doesn't work with pawns:

-for_each( piece of other color ) --king's move &= (~enemy's move)

So that's the basics of what I changed

LB-- commented 11 years ago

Just a note before I respond to anything: I want to get styleguide conformance before I start refactoring the code.

Something I did that I am open to changing (or at least, feel I need to say I'm open about) is the fact that AppStateGame knows what piece is selected and is "current". I did this because I felt that these are things that are results of the user clicking the mouse and didn't belong in a logic based class.

There should be a Player class, which AIPlayer, NetworkPlayer, and HumanPlayer all extend - a player would maintain this and control pieces.

There is no more inheritance. I feel it makes it more complicated than it has to be, particularly when it comes to checking for moves that make check and blocking moves that might cause check.

How so? I think that inheritance is a great path for modularity - we could easily add new and different pieces if we wanted (or if someone forking the project wanted) though for now I think we should get Vanilla Chess working before we go for modded chess.

A piece is really a uint32_t with various masks for it's members, easily changeable if wanted.

The repo is called "ChessPlusPlus", not "cchess" - the goal of the project is to show the power and elegance of C++, not to show how to obfuscate code and use C ;)

A piece's valid moves is no longer a list (so called), but a std:bitset. The list implementation was noticeably slow when it came to dealing with check.

That's no reason to obfuscate code and revert to C techniques. We could easily optimize the code in a way that is still very clear as to what the code is actually doing - the moment you touch bits you create obfuscated code that is non-modular and confusing to think about.

Rule: design first, optimize later. At least, that's how everyone else seems to do it successfully :)

My Ideas

In terms of logic, I think most logic should be done server-side, which means that when playing online and not via a local 'fake' server, you're limited by connection speed anyway.

A couple heiarchies are needed:

The Logic class is a proxy to the game logic manager, which may be the game itself in singleplayer against AI or two-player (LocalLogic), or it may be an actual server across the network (NetworkLogic).

For full modularity, the Logic class shall basically be a direct I/O class that takes as input the desied actions of a Player and gives as output the resultant display one the game board. This would allow a server to interpret user input in a completely different way or even ignore it, and to at any time display the board in any way it wants, including giving custom move sets and rules for the game.

Eventually, though not right now, a server should be able to send custom dependencies, meaning sounds, graphics, and a different config.json file, meaning that the board setup could be anything. I think that all settings should be centralized in config.json to be sued by VanillaServer and overridden by a network server.

Summary: the client is just a window into what the server wants you to see.

LB-- commented 11 years ago

@Thumperrr: We can discuss things here (it would be better as a concrete record compared to irc anyway). See my post above for my intentions and ideas.

Thumperrr commented 11 years ago

For design choices I agree, but I mean for some smaller things like, browsing over code and thinking "I have no idea what the hell that bit of syntax does.." I'd like to have an IM or something we can do quick back-and-forth's with. There are obviously some things you're more adept at in the C++11 world than I.

Would you have any objections to modifying operator[] on JsonParser to be able to do something like this reader()["chesspp.board.cell_width"] as well as doing this reader()["chesspp"]["board"]["cell_width"]? That's one thing I took away from boost::property_tree that I really liked.

Also, I know I was first against it, but I'm starting to like json-parser over boost::property_tree::json_parser. The thing you did with the board layout inside config.json blew my mind. I don't think boost::property_tree could handle that. you use the auto keyword when using the datatype that contains the board layout. Could you explain what the actual data type is?

Can you include other files within .json files? I think it would be beneficial to have config.json 'include' another json file that contains the board layout.

LB-- commented 11 years ago

Would you have objection to modifying operator[] on JsonParser to instead of doing something like this: reader()["chesspp"]["board"]["cell_width"] You could do something like this: reader()["chesspp.board.cell_width"]

I would object because it is possible that names could contain dots. For example:

{
    "files":
    [
        "a.txt",
        "b.txt"
    ],
    "file info":
    {
        "b.txt": <--valid
        {
            "blah": "meh"
        }
    }
}

That's one thing I took away from boost::property_tree that I really liked.

That's because it is not legal in XML, however it is legal in JSON.

Can you include other files within .json files? I think it would be beneficial to have config.json 'include' another json file that contains the board layout.

I think YAML supports that (YAML supports a lot of things and a valid JSON is a valid YAML), but as for JSON we would have to have a string with a filename. I think we could allow both inline board definition and external-file board definition with a simple type check - I'll work on it.

LB-- commented 11 years ago

Actually, thinking about it, we should actually have different config files for the board setup and its graphics - having the different config classes load the same config file doesn't make much sense to me.

for some smaller things like, browsing over code and thinking "I have no idea what the hell that bit of syntax does.." I'd like to have an IM or something we can do quick back-and-forth's with.

You can go to the commit that added that line of code and ask about it on that line - that way everyone can see.

Thumperrr commented 11 years ago

I would object because it is possible that names could contain dots

Good point. I hadn't thought of that. I withdraw my suggestion.

Actually, thinking about it, we should actually have different config files for the board setup and its graphics

I was about to touch on that. That seems like the most intuitive thing to do.

{
    "files":
    [
        "a.txt",
        "b.txt"
    ],
}

In the above snippet, when you go reader()["files"], what data type would it return?

LB-- commented 11 years ago

In the above snippet, when you go reader()["files"], what data type would it return?

JsonReader::NestedValue, which can be implicitly converted to a string: std::string filename = reader()["files"]; EDIT: See issuecomment-17949483

Thumperrr commented 11 years ago

What would be the value of that string? There are two values in "files". Would it evaluate to "a.txt b.txt"?

LB-- commented 11 years ago

Oh, sorry - I misread. The value of reader()["files"] is JsonReader::NestedValue and .type() == json_array, so you would use [numeric_index] to get to what I was talking about.

auto &arr = reader()["files"];
std::vector<std::string> files;
for(std::size_t i = 0; i < arr.length(); ++i)
{
    files.push_back(arr[i]);
}

There's actually some gaps in the JsonReader interface, one of which is .length() - I'll add the missing things soon.

LB-- commented 11 years ago

Added missing capabilities in https://github.com/LB--/ChessPlusPlus/commit/dfd7c173d3f844515ab99ddb3aafc21583c0708b and also abstracted the code where I had to use .implementation(). I had to use some template varargs magic to make it work though :p

LB-- commented 10 years ago

I'm still chugging away refactoring the code in my fork. The main thing I'm having a hard time on is Interactions, I've basically gotten nearly everything but that refactored. Then I'll just need to make the game playable and turn-based.

After that's done I can send a PR and start working on separating client and server logic (which will lead into multiplayer). But before I can send the PR I need to wait for the CMake stuff to get finished and pulled in.

So, this is just a status update for everyone. I'm still working on this :)