Skip to content
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

Merged
merged 13 commits into from
Oct 5, 2024
Merged

Tictactoe #144

merged 13 commits into from
Oct 5, 2024

Conversation

fschledorn
Copy link
Collaborator

@fschledorn fschledorn commented Sep 29, 2024

The main changes are:

  • removed tictactoe.o files, the erstis implement the functions themselves now
  • 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
  • adjusted makefile
  • added a solution for tictactoe

The build process is supposed to fail at this stage. It will only build later after all chapters have been updated

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
@fschledorn fschledorn added the fix_before_vorkurs25 Sollte am besten vor dem Vorkurs in 2025 gefixt werden label Sep 29, 2024
Copy link
Contributor

@Plebysnacc Plebysnacc left a 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

files/solutions/tictactoe.cpp Outdated Show resolved Hide resolved
files/solutions/tictactoe.cpp Show resolved Hide resolved
files/tictactoe.cpp Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
@fschledorn fschledorn requested a review from calvalre September 30, 2024 09:28
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
@fschledorn fschledorn self-assigned this Sep 30, 2024
Copy link

@calvalre calvalre left a 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

files/tictactoe.cpp Show resolved Hide resolved
lektionen/tictactoe.tex Outdated Show resolved Hide resolved
Copy link

@calvalre calvalre left a 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
@Plebysnacc
Copy link
Contributor

will do review tmrrw


int main() {
// Setup
std::array<int, 9> spielfeld{0};
Copy link
Collaborator Author

@fschledorn fschledorn Oct 3, 2024

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

Copy link
Contributor

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};
Copy link
Contributor

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

@Plebysnacc
Copy link
Contributor

lgtm :)

@fschledorn fschledorn merged commit ee3f40a into new_features_2025 Oct 5, 2024
1 check failed
@fschledorn fschledorn deleted the tictactoe branch October 5, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix_before_vorkurs25 Sollte am besten vor dem Vorkurs in 2025 gefixt werden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants