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.

Sign up for GitHub

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

Jump to bottom

hugo/feature/Add ColorBingo #429

Merged
merged 3 commits into from
Nov 9, 2023
Merged

hugo/feature/Add ColorBingo #429

merged 3 commits into from
Nov 9, 2023

Conversation

HPezz
Copy link
Contributor

@HPezz HPezz commented Nov 6, 2023

  • ⬆️ (GameEngineKitExample): Add NSBluetoothAlwaysUsageDescription to Info.plist
  • ✨ (GameEngineKit): Add RobotThenTouchToSelect views
  • ✨ (GameEngineKit): Add ColorBingo activity

@HPezz HPezz requested a review from ladislas November 6, 2023 17:17
@HPezz HPezz self-assigned this Nov 6, 2023
@HPezz HPezz linked an issue Nov 6, 2023 that may be closed by this pull request
Specific YAML - Add Color Bingo @HPezz #430
Closed
ladislas
ladislas requested changes Nov 6, 2023
View reviewed changes
Copy link
Member

@ladislas ladislas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ça m'a l'air bien mais j'ai pas vraiment pu tester parce que tout crash :(

Apps/GameEngineKitExample/Project.swift Outdated Show resolved Hide resolved
...Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView+1_OneChoice.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
@HPezz HPezz force-pushed the hugo/feature/Add-ColorBingo branch 2 times, most recently from 723943a to 4688639 Compare November 7, 2023 11:07
ladislas
ladislas requested changes Nov 7, 2023
View reviewed changes
Copy link
Member

@ladislas ladislas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

très intéressant, ça ouvre pas mal de questions que j'ai mises en commentaires.

j'apprécie beaucoup qu'on ait réussi à prendre de la hauteur et à développer les briques pour faire une activité qu'on appelle Color Bingo mais qui est constitué de choses très simples

même pour cette activité spécifique on voit qu'on a un potentiel de généralisation très important et ça c'est génial!

...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
@HPezz HPezz force-pushed the hugo/feature/Add-ColorBingo branch 2 times, most recently from 72b3d18 to 338299e Compare November 8, 2023 15:25
ladislas
ladislas requested changes Nov 8, 2023
View reviewed changes
Copy link
Member

@ladislas ladislas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deuxième review, on y est presque 👍
j'ai l'impression qu'on utilise pas le nouveau action

Modules/ContentKit/Sources/Activity.swift
Comment on lines +56 to 58
case robotThenTouchToSelect
case listenThenTouchToSelect
case observeThenTouchToSelect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est-ce qu'on mettrait pas tout ça dans case actionThenTouchToSelect

on aura aussi dans un futur proche case actionThenDranAndDrop, case actionThenAssociation etc.

ça évite d'en rajouter trois à la fois. surtout que l'action est dans le payload avec déjà son type.

on peut faire cette modification dans une future PR ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

...itExample/Resources/Assets.xcassets/pictogram_leka_action.imageset/pictogram_leka_action.svg Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

je suis pas fan de cette image du tout :(

le petit jaune qui brille que tu avais dans la vidéo est beaucoup mieux.

ou alors l'image de connection aux BLE

mais celui là il me terrifie et les trucs autour font plus penser à de la musique, de la télépathie ou des ondes très dangereuses qu'à un truc que va faire le robot

...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
...meEngineKit/Sources/_NewSystem/Views/RobotThenTouchToSelect/RobotThenTouchToSelectView.swift Outdated Show resolved Hide resolved
@HPezz HPezz force-pushed the hugo/feature/Add-ColorBingo branch from 338299e to 3228aa8 Compare November 9, 2023 09:47
@HPezz HPezz force-pushed the hugo/feature/Add-ColorBingo branch from 3228aa8 to b7605be Compare November 9, 2023 10:07
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Nov 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ladislas ladislas merged commit d5ffd1a into main Nov 9, 2023
5 of 6 checks passed
@ladislas ladislas deleted the hugo/feature/Add-ColorBingo branch November 9, 2023 10:21
@ladislas ladislas mentioned this pull request Nov 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ladislas ladislas ladislas requested changes

Assignees

@HPezz HPezz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Specific YAML - Add Color Bingo @HPezz
2 participants
@HPezz @ladislas