-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tictactoe #144
Tictactoe #144
Conversation
The main changes are: - removed tictactoe.o files - rewrote the lection and combined the old chapters 17 and 19 to one - modified the tictactoe.cpp file to reflect the desing changes in the lection - tbd: adjust makefile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
solution may be discussed with others again, we should either use introduced techniques, or add these techniques to the tutorial.
Might also wanna add easier to read names
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
Co-authored-by: Tim Grube <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the .tex has that funky paragraph with missing words, and we should decide whether we keep the switch statements and so or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
…s under g++ not clang/llvm for some reason I have to find out in a second
will do review tmrrw |
|
||
int main() { | ||
// Setup | ||
std::array<int, 9> spielfeld{0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does currently not compile on Apple LLVM/Clang as they have not yet fully implemented the C++ 20 Standard. As normal LLVM-Clang has implemented aggregate Initialisation as far back as 2022 and g++ supports this as well, I have decided to ignore this issue.
A possible fix could be to just initialise it using a for-loop, but I’m lazy so instead of fixing this by writing 3 lines I will write 20 lines ranting here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd open an Issue for this and wait how far the homebrew clang got next year
|
||
int main() { | ||
// Setup | ||
std::array<int, 9> spielfeld{0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd open an Issue for this and wait how far the homebrew clang got next year
lgtm :) |
The main changes are:
lection
The build process is supposed to fail at this stage. It will only build later after all chapters have been updated