OOP Error | Together C & C++ | Page 1

civic zinc May 10, 2024, 1:33 PM

#

Hi!

Can someone help me check what is going on with my code?

class Piece{
    protected:
        shared_ptr<Cell> cell;
        bool skipObstacles;
    
        void MoveToCell(shared_ptr<Cell> startingCell);

    public:
        Faction faction;
        PieceType type;

        Piece(shared_ptr<Cell> startingCell, Faction faction, PieceType type, bool skipObstacles);
        /// @brief define the basic ruleset of a particular chess piece regardless of obstacles or legality
        virtual void SetPieceOnCell() = 0;
        virtual vector<int> GetPieceDefaultPath() = 0;
};

class King : public Piece{
    public:
        void SetPieceOnCell() override;
        King(shared_ptr<Cell> startingCell, Faction faction): Piece(startingCell, faction, KING, false) {}
        vector<int> GetPieceDefaultPath() override;
};

King has implementation of SetPieceOnCell method as follows:

void King::SetPieceOnCell(){
    shared_ptr<King> my_ptr(this);
    this->cell->SetPieceOnCell(my_ptr);
}

In Piece, there is a method called MoveToCell that has the following implementation

void Piece::MoveToCell(shared_ptr<Cell> startingCell){
    if(this->cell != nullptr){
        this->cell->SetCellEmpty();
    }
    this->cell = nullptr;
    this->cell = startingCell;
    this->SetPieceOnCell();
}

for some reason, SetPieceOnCell is calling virtual method defined in the base class instead of the derived class

    vector<shared_ptr<Piece>> white_pieces;
    vector<shared_ptr<Piece>> black_pieces;

    white_pieces.emplace_back(make_shared<King>(King(cells[4][7], Faction::WHITE_FACTION)));

onyx rapidsBOT May 10, 2024, 1:33 PM

#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

glossy horizon May 10, 2024, 1:36 PM

#

Is SetPieceOnCell meant to take a parameter?

#

It would be good if you could provice an mcve on godbolt or similar.

civic zinc May 10, 2024, 1:38 PM

#

glossy horizon It would be good if you could provice an [mcve](<https://stackoverflow.com/help/...

mcve?

#

oh wait

#

Oh nvm

glossy horizon May 10, 2024, 1:41 PM

#

No. There is no way I can check if a change fixes the problem given a video.

#

At least not with unreasonable effort of typing it all out.

civic zinc May 10, 2024, 1:42 PM

#

does sharing repo work?

glossy horizon May 10, 2024, 1:43 PM

#

Does it show the error you have?

lavish plover May 10, 2024, 1:49 PM

#

civic zinc Hi! Can someone help me check what is going on with my code? ```cpp class Piec...

shared_ptr<King> my_ptr(this); is utterly cursed

#

And doesn't work

#

Or more like it will almost certainly break things

civic zinc May 10, 2024, 1:50 PM

#

I just quickly created this

#include <memory>
#include <iostream>

class ClassA{
    public:
        virtual void overrideme() = 0;
        ClassA();
};

class ClassB : public ClassA{
    public:
        void overrideme() override;
        ClassB();
};

ClassA::ClassA(){
    std::cout << "printing from Class A" << std::endl;
};

ClassB::ClassB() : ClassA(){
    std::cout << "printing from Class B" << std::endl;
};

void ClassB::overrideme(){
    std::cout << "override method from Class B" << std::endl;
}

int main(){
    std::shared_ptr<ClassA> a_ptr = std::dynamic_pointer_cast<ClassA>(std::make_shared<ClassB>());
    a_ptr->overrideme();
    return 0;
}

This is my intention and this works

civic zinc May 10, 2024, 1:51 PM

#

lavish plover `shared_ptr<King> my_ptr(this);` is utterly cursed

Oh

lavish plover May 10, 2024, 1:51 PM

#

The constructor for shared ptr that takes a pointer basically assumes that it can safely take ownership of the pointed object, and that that object was created with new

#

There's generally no guarantee that that's true out of context

#

Considering how many shared ptr you have around in your code, let's say that the object was actually owned by a shared ptr initially, then that piece of code would still be broken

#

Because the newly created shared ptr would have no knowledge of the other existing shared ptr

#

So it would go on its merry way to setup a new, different, control block/shared counter

#

So you'd end up with a different shared counter which would go and (try to) destroy the object independently based on the number of shared owners they separately keep track of

#

There are decent odds that after your function call the local shared ptr in that function just nukes the current object

#

Anyway if you want to enable this kind of code, you'd have to turn to something like "enable shared from this", though honestly I'd rather structure the thing with unique ptr and non owning "references" instead

civic zinc May 10, 2024, 2:01 PM

#

lavish plover Anyway if you want to enable this kind of code, you'd have to turn to something ...

non owning "references" mean...raw pointers?

#

Ig using smart pointers do not make me smart 💀

#

Oh actual references

lavish plover May 10, 2024, 2:02 PM

#

In a codebase where smart pointers are used consistently, raw pointers are generally understood to never own the pointed object, yes

#

Actual references can work depending on the exact context

civic zinc May 10, 2024, 2:05 PM

#

I will try to convert them into references if that's the case, thanks!

#

However

#

This part of the code is never triggered after all

#

Hi

civic zinc May 10, 2024, 2:07 PM

#

glossy horizon It would be good if you could provice an [mcve](<https://stackoverflow.com/help/...

Here is the mcve

#

#include <memory>
#include <iostream>

class ClassA{
    public:
        virtual void overrideme() = 0;
        ClassA();
};

class ClassB : public ClassA{
    public:
        void overrideme() override;
        ClassB();
};

ClassA::ClassA(){
    std::cout << "printing from Class A" << std::endl;
    this->overrideme();
};

ClassB::ClassB() : ClassA(){
    std::cout << "printing from Class B" << std::endl;
};

void ClassB::overrideme(){
    std::cout << "override method from Class B" << std::endl;
}

int main(){
    std::shared_ptr<ClassA> a_ptr = std::dynamic_pointer_cast<ClassA>(std::make_shared<ClassB>());
    a_ptr->overrideme();
    return 0;
}

#

The main issue is here

ClassA::ClassA(){
    std::cout << "printing from Class A" << std::endl;
    this->overrideme();
};

I am trying to call the virtual method here

this->overrideme

lavish plover May 10, 2024, 2:08 PM

#

That's also never going to work

glossy horizon May 10, 2024, 2:08 PM

#

Inside ClassA::ClassA there is no ClassB yet.

civic zinc May 10, 2024, 2:08 PM

#

That makes sense

#

So, for each Derived Class implementation I might need to have its own constructor if I want to trigger the virtual method?

glossy horizon May 10, 2024, 2:11 PM

#

I'm not sure what you want to achieve. You can probably get around it by calling it not in the constructor (or destructor).

civic zinc May 10, 2024, 2:14 PM

#

Ye, I just realized this is massively flawed...

civic zinc May 10, 2024, 2:15 PM

#

lavish plover Anyway if you want to enable this kind of code, you'd have to turn to something ...

I think I will try to do this first

#

Thanks either way!

onyx rapidsBOT May 10, 2024, 2:20 PM

#

@civic zinc Has your question been resolved? If so, type !solved :)

tender quartz May 10, 2024, 3:03 PM

#

this is the most complicated way anyone has ever programmed chess i think 😮

civic zinc May 10, 2024, 3:09 PM

#

tender quartz this is the most complicated way anyone has ever programmed chess i think 😮

Really??

#

Am I overengineering

tender quartz May 10, 2024, 3:09 PM

#

definitely

civic zinc May 10, 2024, 3:09 PM

#

💀

tender quartz May 10, 2024, 3:10 PM

#

and depending on how much performance you want that as well

glossy horizon May 10, 2024, 3:10 PM

#

Generally chess engines need to be fast and you do not need any virtual functions at all.

tender quartz May 10, 2024, 3:10 PM

#

well i think in this case it's a gui

#

ah nvm the file is named chess-engine.cpp

civic zinc May 10, 2024, 3:11 PM

#

Ye

#

glossy horizon May 10, 2024, 3:11 PM

#

But I don't think making a good chess engine is your goal. If it was you'd use Stockfish. The goal is to learn some C++ and program design, which you're doing. That the resulting chess engine cannot compete with Stockfish doesn't matter.

civic zinc May 10, 2024, 3:11 PM

#

glossy horizon But I don't think making a good chess engine is your goal. If it was you'd use S...

Ofc not 💀

#

Not my expectation

tender quartz May 10, 2024, 3:13 PM

#

well many people write their own good chess engine.. just because they like trying you dont necessarily have to aim for the top but you can get a good one nonetheless

civic zinc May 10, 2024, 3:13 PM

#

!solved

onyx rapidsBOT May 10, 2024, 3:13 PM

#

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

civic zinc May 10, 2024, 3:14 PM

#

tender quartz well many people write their own good chess engine.. just because they like tryi...

Yee

tender quartz May 10, 2024, 3:16 PM

#

civic zinc Am I overengineering

to mention a few things,
a) inheritance is overkill for pieces
b) shared_ptr here also looks very weird
c) defining the board as a nested array is likely bad performance wise, just make a 1d 64 square array
d) why does a piece have to know about obstacles? encoding the current pieces position in the piece itself is weird
e) dont use vectors

civic zinc May 10, 2024, 3:18 PM

#

tender quartz to mention a few things, a) inheritance is overkill for pieces b) shared_ptr he...

So for a) instead of creating separate classes for each Piece Type, just have the procedures inside a method in Piece class and do things based on switch case?

#

b) I will try to redesign

#

c) sure, I will implement a translator method to map things in that case

tender quartz May 10, 2024, 3:19 PM

#

a translator method ? uff 😛

civic zinc May 10, 2024, 3:19 PM

#

d) it was for the knight, knight skip over obstacles rt while others cannot

#

Or maybe that is not necessary 👀

#

e) okay

tender quartz May 10, 2024, 3:20 PM

#

civic zinc So for a) instead of creating separate classes for each Piece Type, just have th...

you can have a general piece class which is based on enums,
i.e. stockfish

enum Piece {
    NO_PIECE,
    W_PAWN = PAWN,     W_KNIGHT, W_BISHOP, W_ROOK, W_QUEEN, W_KING,
    B_PAWN = PAWN + 8, B_KNIGHT, B_BISHOP, B_ROOK, B_QUEEN, B_KING,
    PIECE_NB = 16
};

generally saying i try to keep the pieces detached from the current board state completely

civic zinc May 10, 2024, 3:21 PM

#

tender quartz a translator method ? uff 😛

Just to sound a tad more professional

#

XF

tender quartz May 10, 2024, 3:22 PM

#

tender quartz you can have a general piece class which is based on enums, i.e. stockfish ```cp...

in my own chess library i have a wrapper around my enums

class Piece {
   public:
    enum class underlying : std::uint8_t {
        WHITEPAWN,
        WHITEKNIGHT,
        WHITEBISHOP,
        WHITEROOK,
        WHITEQUEEN,
        WHITEKING,
        BLACKPAWN,
        BLACKKNIGHT,
        BLACKBISHOP,
        BLACKROOK,
        BLACKQUEEN,
        BLACKKING,
        NONE
    };
    
    // some other helper functions
}

civic zinc May 10, 2024, 3:22 PM

#

tender quartz you can have a general piece class which is based on enums, i.e. stockfish ```cp...

Okay, then how does it refer back to the board then?

#

Oh

#

It doesn't

civic zinc May 10, 2024, 3:22 PM

#

tender quartz in my own chess library i have a wrapper around my enums ```cpp class Piece { ...

Why is it duplicated

tender quartz May 10, 2024, 3:23 PM

#

std::array<Piece, 64> board_       = {};

tender quartz May 10, 2024, 3:23 PM

#

civic zinc Why is it duplicated

what is duplicated?

civic zinc May 10, 2024, 3:23 PM

#

Got one WhitePawn and BlackPawn

#

So this particular enum manages both faction and pieceType?

#

Instead of having them separated

glossy horizon May 10, 2024, 3:24 PM

#

It doesn't manage anything. It simply enumerates all the pieces.

civic zinc May 10, 2024, 3:24 PM

#

Oh

glossy horizon May 10, 2024, 3:25 PM

#

If you reorder a bit you can use the last bit to get the color.

#

Or even the score of the piece.

tender quartz May 10, 2024, 3:25 PM

#

glossy horizon Or even the score of the piece.

score?

tender quartz May 10, 2024, 3:26 PM

#

tender quartz you can have a general piece class which is based on enums, i.e. stockfish ```cp...

well this is the case here in Stockfish or pretty much most engines
piece & 7 will get you the color, where 0 is black and 1 white

glossy horizon May 10, 2024, 3:26 PM

#

The typical way is to assign a score to each piece. Pawn is 1, queen is 9, bishop 4 (I think). Then you just go through all possible moves. If your score minus opponent score goes up it's a good move, otherwise it's a bad move.

#

It's a bit primitive, but it quickly gives you something that kinda sorta is somewhat smart.

tender quartz May 10, 2024, 3:27 PM

#

well.. in a primitive engine maybe, but no one has it that way

glossy horizon May 10, 2024, 3:28 PM

#

Pretty sure Stockfish does this. They just additionally have scores for positions which is more complicated.

#

It's even exposed. When looking at Stockfish analysis it tells you the scoring of each move.

#

And explains why it's a good/bad move by showing you where the increase/decrease of the score comes from.

tender quartz May 10, 2024, 3:29 PM

#

Well I am one of the Stockfish maintainers and no 😛
The score is not attached to the piece it is just a look up in a separate array

constexpr Value PieceValue[PIECE_NB] = {
  VALUE_ZERO, PawnValue, KnightValue, BishopValue, RookValue, QueenValue, VALUE_ZERO, VALUE_ZERO,
  VALUE_ZERO, PawnValue, KnightValue, BishopValue, RookValue, QueenValue, VALUE_ZERO, VALUE_ZERO};

tender quartz May 10, 2024, 3:31 PM

#

glossy horizon Pretty sure Stockfish does this. They just additionally have scores for position...

complicated is a vague term and additional scores is pretty much no longer a thing since it is all neural networks now

tender quartz May 10, 2024, 3:31 PM

#

glossy horizon And explains why it's a good/bad move by showing you where the increase/decrease...

the "explaining" is not something which comes from stockfish, lichess/chess.com have their own logic for that

glossy horizon May 10, 2024, 3:32 PM

#

tender quartz Well I am one of the Stockfish maintainers and no 😛 The score is not attached ...

My suggestion was to join that with the enum value. It's probably not that useful in practice, but kinda neat.

#

Also neat that you're maintaining Stockfish.

#

Not so neat that it's just neural networks, but what can you do.

tender quartz May 10, 2024, 3:34 PM

#

well we got rid of it for a reason, simply that neural networks are better

#

though stockfish has a different network than lc0 for example, lc0 has much more expensive to compute network which runs on the gpu and is also deeper
whereas stockfish's network can be efficiently computed on the cpu and is way faster

tender quartz May 10, 2024, 3:37 PM

#

glossy horizon My suggestion was to join that with the enum value. It's probably not that usefu...

perhaps for a primitive one but as soon as you want to tweak the piece values you will realize that you cant do that with that approach

glossy horizon May 10, 2024, 3:37 PM

#

Yeah, can't have duplicates or fractions either.

#

But but but ... it's so neat to compress it into the enum 😛

tender quartz May 10, 2024, 3:38 PM

#

😄

civic zinc May 10, 2024, 3:38 PM

#

Hmm 👀

#

Interesting

#

Lemme try that! :4

glossy horizon May 10, 2024, 3:39 PM

#

You can probably make a function that computes the actual values, but it's also probably slower than just doing the lookup. Rip.

civic zinc May 10, 2024, 6:28 PM

#

Hello!

#

Thank you ppl

#

I changed it to this

#

#pragma once

#include <memory>
#include <array>
#include <vector>
#include <cstdint>

using std::shared_ptr, std::make_shared, std::unique_ptr, std::make_unique, std::array, std::vector, std::uint8_t;

/// @brief all the different possible piece type
enum class PieceType: uint8_t{
    NO_PIECE,
    W_KING = 1, W_QUEEN, W_BISHOP, W_KNIGHT, W_ROOK, W_PAWN,
    B_KING = 9, B_QUEEN, B_BISHOP, B_KNIGHT, B_ROOK, B_PAWN
};

class Piece;
class Cell;
class Board;

/// @brief the piece class emcompassing all the methods and functionalities
class Piece{
    public:
        PieceType type;
        
        Piece(PieceType type);

        ///@brief check whether if a piece is white or black
        ///@return true -> White Piece, false -> Black Piece
        bool IsWhite();
};

/// @brief the cell class that is going to house piece and be inside a board
class Cell{
    unique_ptr<Piece> pieceOnCell;

    public:
        Cell();

        /// @brief get the current piece on the cell
        /// @return pointer to the current piece
        void GetPieceOnCell(Piece*& piecePtr);
        
        /// @brief get the unique ptr to be moved
        /// @return 
        void GetPieceOnCell(unique_ptr<Piece>*& piecePtr);

        /// @brief new piece
        /// @param newPieceOnCell 
        void SetPieceOnCell(Piece newPieceOnCell);

        /// @brief release the previous reference to the piece on this cell and replace it with a new one
        /// @param pieceOnCell current piece on this cell
        void SetPieceOnCell(unique_ptr<Piece>* newPieceOnCell);
};

class Board{
    public:
        struct Coordinate{
            uint8_t X: 4;
            uint8_t Y: 4;
        };

        array<shared_ptr<Cell>, 64> cells;

        Board();

        Cell* GetCell(Coordinate coordinate);

        static uint8_t GetCoordinate(Coordinate coordinate);
};

#

this is before

    white_pieces.emplace_back(cast_to_piece(make_shared<King>(King(cells[4][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Queen>(Queen(cells[3][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Rook>(Rook(cells[0][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Rook>(Rook(cells[7][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Knight>(Knight(cells[1][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Knight>(Knight(cells[6][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Bishop>(Bishop(cells[2][7], Faction::WHITE_FACTION))));
    white_pieces.emplace_back(cast_to_piece(make_shared<Bishop>(Bishop(cells[5][7], Faction::WHITE_FACTION))));

    black_pieces.emplace_back(cast_to_piece(make_shared<King>(King(cells[4][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Queen>(Queen(cells[3][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Rook>(Rook(cells[0][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Rook>(Rook(cells[7][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Knight>(Knight(cells[1][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Knight>(Knight(cells[6][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Bishop>(Bishop(cells[2][0], Faction::BLACK_FACTION))));
    black_pieces.emplace_back(cast_to_piece(make_shared<Bishop>(Bishop(cells[5][0], Faction::BLACK_FACTION))));

glossy horizon May 10, 2024, 6:33 PM

#

You should not have to name the piece type twice.

civic zinc May 10, 2024, 6:33 PM

#

glossy horizon You should not have to name the piece type twice.

Sorry?

glossy horizon May 10, 2024, 6:34 PM

#

black_pieces.emplace_back(cast_to_piece(make_shared<King>(~~King(~~cells[4][0], Faction::BLACK_FACTION~~)~~)));

civic zinc May 10, 2024, 6:35 PM

#

    this->cells[GetCoordinate(Coordinate{4, 7})]->SetPieceOnCell(Piece(PieceType::W_KING));
    this->cells[GetCoordinate(Coordinate{3, 7})]->SetPieceOnCell(Piece(PieceType::W_QUEEN));

this is after

civic zinc May 10, 2024, 6:36 PM

#

glossy horizon black_pieces.emplace_back(cast_to_piece(make_shared<King>(~~King(~~cells[4][0], ...

Ahh

#

yea, I could have just passed the arguments

#

Like now

glossy horizon May 10, 2024, 6:36 PM

#

civic zinc ```cpp this->cells[GetCoordinate(Coordinate{4, 7})]->SetPieceOnCell(Piece(Pi...

You can probably leave out Piece here.

civic zinc May 10, 2024, 6:37 PM

#

void Cell::SetPieceOnCell(Piece newPieceOnCell){
    this->pieceOnCell = make_unique<Piece>(newPieceOnCell);
}

void Cell::SetPieceOnCell(unique_ptr<Piece>* newPieceOnCell){
    this->pieceOnCell = std::move(*newPieceOnCell);
}

#

this is my SetPieceOnCell implementation

#

How can I leave out?

glossy horizon May 10, 2024, 6:37 PM

#

GetCoordinate(Coordinate{3, 7}) feels quite long too.

#

cells[GetCoordinate(Coordinate{4, 7})]->SetPieceOnCell(PieceType::W_KING);

#

And maybe even
cells[GetCoordinate({4, 7}]->SetPieceOnCell(PieceType::W_KING);

#

Though I'd change it so that cells[{4, 7}] works.

#

Or maybe cells[4][7]

civic zinc May 10, 2024, 6:42 PM

#

glossy horizon Or maybe `cells[4][7]`

As mentioned by Disservin, I turnt it into a 1D array actually

#

And yes, I just switched it into

this->cells[GetCoordinate(4, 7)]->SetPieceOnCell(Piece(PieceType::W_KING));

glossy horizon May 10, 2024, 6:43 PM

#

Are you sure Piece( is necessary here?

civic zinc May 10, 2024, 6:43 PM

#

glossy horizon Are you sure `Piece(` is necessary here?

hmm

#

void Cell::SetPieceOnCell(unique_ptr<Piece>* newPieceOnCell){
    this->pieceOnCell = std::move(*newPieceOnCell);
}

How can I pass it to here?

#

std::move(make_unique<Piece>(PieceType::W_KING)) might make it un-necessary

glossy horizon May 10, 2024, 6:45 PM

#

You should not pass a pointer to a unique pointer.

#

void Cell::SetPieceOnCell(unique_ptr<Piece> newPieceOnCell){
    this->pieceOnCell = std::move(newPieceOnCell);
}

civic zinc May 10, 2024, 6:46 PM

#

Ye

#

Why did I do that again?...

#

this->cells[GetCoordinate(4, 7)]->SetPieceOnCell(make_unique<Piece>(PieceType::W_KING));

#

If i remember correctly, I think I was trying to make it not get destroyed when it was being moved between cells

#

Anyhow

#OOP Error