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

Buttons fixes #1193

Merged
merged 6 commits into from
Nov 27, 2023
Merged

Buttons fixes #1193

merged 6 commits into from
Nov 27, 2023

Conversation

AngieDutra
Copy link
Contributor

@AngieDutra AngieDutra commented Nov 24, 2023

Motivation

Buttons in Battle.unity were broken:

  • Exit buttons (in settings and death splash)
  • Settings button
  • Toggles:
    • Client prediction
    • Client prediction ghost
    • Interpolation

Summary of changes

This PR fixes all the buttons listed before. However I recommend the reviewer goes though all the buttons in that scene to check none were broken in the process.
Changes in Battle.unity were necessary since the prefab override, vital for the buttons to work again, implied changes in that scene. Also some references in the scene were missing so adding those also implied changes there.

Checklist

  • I have tested the changes locally.
  • I self-reviewed the changes on GitHub, line by line.
  • Tests have been added/updated.
  • This change requires new documentation.
    • Documentation has been added/updated.
  • I have tested the changes in another devices.
    • Tested in iOS.
    • Tested in Android.

Copy link
Contributor

@ncontinanza ncontinanza left a comment

Choose a reason for hiding this comment

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

LGTM! Tested in Android, every button worked fine.

Copy link
Contributor

@mFragaBA mFragaBA left a comment

Choose a reason for hiding this comment

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

when testing on Unity, I'm getting this error when trying to click on the characters button (I also tried merging with main and then re-testing and still the error pops up):

Cannot load scene: Invalid scene name (empty string) and invalid build index -1
UnityEngine.SceneManagement.SceneManager:LoadScene (string)
MoreMountains.Tools.MMLoadScene:LoadScene () (at Assets/ThirdParty/TopDownEngine/ThirdParty/MoreMountains/MMTools/Tools/MMSceneLoading/Scripts/Helpers/MMLoadScene.cs:34)
UnityEngine.Events.UnityEvent:Invoke ()

The button has no scene set:

image

@AngieDutra AngieDutra marked this pull request as draft November 27, 2023 13:46
@BertovDev
Copy link
Contributor

when testing on Unity, I'm getting this error when trying to click on the characters button (I also tried merging with main and then re-testing and still the error pops up):

Cannot load scene: Invalid scene name (empty string) and invalid build index -1
UnityEngine.SceneManagement.SceneManager:LoadScene (string)
MoreMountains.Tools.MMLoadScene:LoadScene () (at Assets/ThirdParty/TopDownEngine/ThirdParty/MoreMountains/MMTools/Tools/MMSceneLoading/Scripts/Helpers/MMLoadScene.cs:34)
UnityEngine.Events.UnityEvent:Invoke ()

The button has no scene set:
image

@mFragaBA Do you know if this happened after re-imporing the client Engine folder? I have seen the same problem but after those changes.

@mFragaBA
Copy link
Contributor

when testing on Unity, I'm getting this error when trying to click on the characters button (I also tried merging with main and then re-testing and still the error pops up):

Cannot load scene: Invalid scene name (empty string) and invalid build index -1
UnityEngine.SceneManagement.SceneManager:LoadScene (string)
MoreMountains.Tools.MMLoadScene:LoadScene () (at Assets/ThirdParty/TopDownEngine/ThirdParty/MoreMountains/MMTools/Tools/MMSceneLoading/Scripts/Helpers/MMLoadScene.cs:34)
UnityEngine.Events.UnityEvent:Invoke ()

The button has no scene set:
image

@mFragaBA Do you know if this happened after re-imporing the client Engine folder? I have seen the same problem but after those changes.

@BertovDev what would re-importing the client Engine folder? The topdownengine folder? I did recently re-import it. However, I tried deleting the directory from client/Assets/ThirdParty and re-importing it but it was still not working. Also tried closing and re-opening unity but to no avail.

@AngieDutra AngieDutra marked this pull request as ready for review November 27, 2023 16:42
@AngieDutra AngieDutra dismissed mFragaBA’s stale review November 27, 2023 17:29

Talked to the team and it is actually working fine. I'll help you fix this but the team needs this fix so this dismiss was requested.

@AngieDutra AngieDutra merged commit 6bfb1ce into main Nov 27, 2023
1 check passed
@AngieDutra AngieDutra deleted the button-fixes branch November 27, 2023 17:30
fkrause98 pushed a commit that referenced this pull request Nov 28, 2023
* fix settings and exit buttons

* fix toggles

* client prediction toggle fix

* fix interpolation and ghost toggles

* main load scene to be fix

* main load scene fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants