WeSign/unity-application
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 6 Wiki Activity

Resolve WES-81 "Justsign" #46

Closed
lvrossem wants to merge 10 commits from WES-81-justsign into development
pull from: WES-81-justsign
merge into: WeSign:development
Conversation 12 Commits 10 Files Changed 276 +6327 -27
lvrossem commented 2023-03-16 18:43:39 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

Description

This merge request aims to add a prototype the JustSign minigame

Testing Instructions

The game is accessible through the minigame menu. Feel free to test it and see if you encounter any crashes/bugs.

Once you've tested the changes, please confirm that they work as expected and that there are no regressions or unexpected side effects. If any issues are discovered during testing, please include detailed steps to reproduce the issue in the merge request comments. Thank you!

Related Issues

Please list any related issues or pull requests that are relevant to this merge request.
E.g. WES-XXX-...

Known bugs or issues

Please list any known bugs or issues related to the changes in this merge request.

Checklist

  • I have filled in this template.
  • I have tested my changes thoroughly (both in the editor + build and run (ctrl+B)!).
  • I have added appropriate unit tests.
  • I have updated the user documentation as necessary.
  • Code reviewed by 2 people.

Additional Notes

Please add any additional notes or comments that may be helpful for reviewers to understand your changes.

Closes WES-81

## Description This merge request aims to add a prototype the JustSign minigame ## Testing Instructions The game is accessible through the minigame menu. Feel free to test it and see if you encounter any crashes/bugs. Once you've tested the changes, please confirm that they work as expected and that there are no regressions or unexpected side effects. If any issues are discovered during testing, please include detailed steps to reproduce the issue in the merge request comments. Thank you! ## Related Issues _Please list any related issues or pull requests that are relevant to this merge request._ _E.g. WES-XXX-..._ ## Known bugs or issues _Please list any known bugs or issues related to the changes in this merge request._ ## Checklist - [X] I have filled in this template. - [X] I have tested my changes thoroughly (both in the editor + **build and run (ctrl+B)**!). - [ ] I have added appropriate unit tests. - [X] I have updated the user documentation as necessary. - [ ] Code reviewed by 2 people. ## Additional Notes _Please add any additional notes or comments that may be helpful for reviewers to understand your changes._ Closes WES-81
lvrossem commented 2023-03-16 18:43:39 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

requested review from @louadria, @tihabils, @dvschuyl, @lvrossem, @hvbreuge, @jeldgees, and @jrcoudro

requested review from @louadria, @tihabils, @dvschuyl, @lvrossem, @hvbreuge, @jeldgees, and @jrcoudro
lvrossem commented 2023-03-16 18:44:07 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

changed target branch from main to development

changed target branch from `main` to `development`
lvrossem commented 2023-03-16 18:45:38 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

added 3 commits

  • 4fdf52b7...5e26970b - 2 commits from branch development
  • 16b13122 - Merge branch 'development' into 'WES-81-justsign'

Compare with previous version

added 3 commits <ul><li>4fdf52b7...5e26970b - 2 commits from branch <code>development</code></li><li>16b13122 - Merge branch &#39;development&#39; into &#39;WES-81-justsign&#39;</li></ul> [Compare with previous version](/wesign/unity-application/-/merge_requests/46/diffs?diff_id=31048&start_sha=4fdf52b7b151be759651e68edaf02c5b58a11a15)
lvrossem commented 2023-03-17 11:30:02 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>b0ccda3a - Add scores to JustSign</li></ul> [Compare with previous version](/wesign/unity-application/-/merge_requests/46/diffs?diff_id=31079&start_sha=16b13122c277c7651b4e8f5d38e3e920b617f025)
lvrossem commented 2023-03-17 13:43:45 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

marked the checklist item I have tested my changes thoroughly (both in the editor + build and run (ctrl+B)!). as completed

marked the checklist item **I have tested my changes thoroughly (both in the editor + **build and run (ctrl+B)**!).** as completed
lvrossem commented 2023-03-17 13:44:13 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

marked this merge request as ready

marked this merge request as **ready**
jrcoudro commented 2023-03-17 14:41:12 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

Looks good generally, few things to note:

  • testing is kind of difficult since the images don't crearly represent what needs to be typed.
  • You can't type a sign if it is not in front, which makes it so that missing one sign makes you miss the next one as well
  • The minigame selection screen doesn't move to the richt info-page. But I managed to get this working for Hangman, so you can leave that be for now
Looks good generally, few things to note: - testing is kind of difficult since the images don't crearly represent what needs to be typed. - You can't type a sign if it is not in front, which makes it so that missing one sign makes you miss the next one as well - The minigame selection screen doesn't move to the richt info-page. But I managed to get this working for Hangman, so you can leave that be for now
louadria commented 2023-03-17 15:25:50 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

Just tested, works as expected. Looks nice!

  • As Jerome noted, it would maybe be nice to add the word you have to type under the image for clarity.
  • The back button is not working
  • Right now it's possible for the same sign to appear twice. Maybe it would be possible to work with 2 lists, where one represents the available signs, and the other the signs that are currently in used.
  • Maybe for a future update, it would be fun if the speed increases per sign? Maybe let the player have a number of hearts, and whenever a sign passes to the end the player loses a heart, and when they reach 0 hearts the game could end. This would be for a future MR, however.
Just tested, works as expected. Looks nice! * As Jerome noted, it would maybe be nice to add the word you have to type under the image for clarity. * The back button is not working * Right now it's possible for the same sign to appear twice. Maybe it would be possible to work with 2 lists, where one represents the available signs, and the other the signs that are currently in used. * Maybe for a future update, it would be fun if the speed increases per sign? Maybe let the player have a number of hearts, and whenever a sign passes to the end the player loses a heart, and when they reach 0 hearts the game could end. This would be for a future MR, however.
jrcoudro commented 2023-03-17 18:25:02 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

Maybe it could be a good idea to add the webcam to this game as well, even if it does nothing just yet

Maybe it could be a good idea to add the webcam to this game as well, even if it does nothing just yet
lvrossem commented 2023-03-18 23:32:46 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>666815db - Apply most requested changes</li></ul> [Compare with previous version](/wesign/unity-application/-/merge_requests/46/diffs?diff_id=31277&start_sha=b0ccda3ab5138ccd325515aa8806ae63ba5920dd)
lvrossem commented 2023-03-18 23:33:51 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

I have applied most of your requested changes, only displaying the word underneath the image turns out to be a headache, so I will figure that one out later.

I have applied most of your requested changes, only displaying the word underneath the image turns out to be a headache, so I will figure that one out later.
lvrossem commented 2023-03-19 13:22:28 +00:00 (Migrated from gitlab.ilabt.imec.be)
Copy Link

added 1 commit

  • cf605ab8 - Add words to symbols in JustSign

Compare with previous version

added 1 commit <ul><li>cf605ab8 - Add words to symbols in JustSign</li></ul> [Compare with previous version](/wesign/unity-application/-/merge_requests/46/diffs?diff_id=31331&start_sha=666815db22ec4c80333e079d650b6654f51ca005)

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
enhancement
impact::high
impact::low
impact::minor
impact::moderate
impact::very low
probability::high
probability::low
probability::minor
probability::moderate
probability::very low
progress::busy
progress::done
progress::review
progress::todo
version::0.1
version::0.2
version::0.3
version::0.4
version::0.5
version::1.0
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
VictorMylle
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: WeSign/unity-application#46
Delete Branch "WES-81-justsign"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?