Creating fade in / fade out black effect at the beginning of the game

med-material / Whack_A_Mole_VR

Whack-A-Mole in VR

MIT License

3 stars 15 forks source link

Creating fade in / fade out black effect at the beginning of the game #195

Closed lucasmnt closed 1 year ago

lucasmnt commented 1 year ago

Adding a script component to the camera rig's child camera called LevelFaderScreen.cs

Script LevelFaderScreen.cs creates fade in / fade out black effect at the start of the game

This script uses built-in SteamVR functions to create the wanted effect

bastianilso commented 1 year ago

Hi @lucasmnt , great to see your first pull request here! 🌴

The commit looks very clean. I just have three comments for you to address:

First, If you could write a commit message that shortly explains what caused the issue and how you fixed it, then it's golden. 🧲
Could you do a quick test of how the fade-out looks like in a standalone Unity build? You can use Screen2Gif to record it and upload an animation here. Thanks!
Please check the code comments coming after this reply.

To address these issues, I would like you to rebase your branch and force-push your branch to update your pull request. Once we approve your pull request at the end of this, I will rebase it on top of our master branch. Let me know if you have any questions regarding this process. 🙂

bastianilso commented 1 year ago

@lucasmnt it looks like, some additional commits were merged into the git branch, which are unrelated to the current pull request we are reviewing. These will need to be removed and treated in a separate pull request (as part of this Git exercise).

once we have this pull request merged, xavier can rebase his work on top of the updated master branch again. 👍

Xav1204 commented 1 year ago

There are also my code about the Screen Capture in the script "ScreenShot.cs"

bastianilso commented 1 year ago

@Xav1204 for the purpose of review, it's better to isolate these two pull requests from each other, as they concern two separate topics. This way we can also discuss the contents separately and there's a clear link from issue to pull request. 🙂

Xav1204 commented 1 year ago

@bastianilso I'm agree with you, it's just that is the first time that we use a fork in collaboration so I thought that I could make a pull request on the fork without changing the lucas's pull request on the med-material Github. We won't make this mistake the next time.

bastianilso commented 1 year ago

@Xav1204 @lucasmnt Both commits look clean.

@Xav1204 for your commit about the screenshots, I would like you to update the README.md file itself as well with the new screenshots. I will rebase and close this pull request now, but you can do this in a separate pull request.