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

feat: Add dropdown menu for profiles #494

Merged
merged 6 commits into from
Aug 8, 2023
Merged

feat: Add dropdown menu for profiles #494

merged 6 commits into from
Aug 8, 2023

Conversation

Jan200101
Copy link
Contributor

add dropdown menu for profiles

@Jan200101
Copy link
Contributor Author

@GeckoEidechse

@Jan200101
Copy link
Contributor Author

Had to fix a small issue where the first commit did not have a missing import (how this evaded me is unknown, tauri dev didn't error on this)

@GeckoEidechse GeckoEidechse changed the title add dropdown menu for profiles feat: Add dropdown menu for profiles Aug 8, 2023
Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

In quick test, switching profile does not updated the detected version in PlayView.

@GeckoEidechse
Copy link
Member

UI wise, there should maybe also be some explainer of what profiles are. Maybe a small ? icon next to Active Profile that if hovered/clicked shows a tooltip or reveals small text that gives a short explainer.

Personally I'd still suggest to gate profile selecting behind DevView for now so that we can skip all these small UI quirks to that are kinda needed to make the feature "end-user compatible" and so that we can already test it with contributors.

Then we could even do a 3.0 release where we basically just move it from DevView to RepairView when deemed stable enough ^^

@Jan200101
Copy link
Contributor Author

Jan200101 commented Aug 8, 2023

In quick test, switching profile does not updated the detected version in PlayView.

I had only picked the first commit, as you requested.
The commits that fix that were cherry picked onto this branch.

@Jan200101 Jan200101 requested a review from GeckoEidechse August 8, 2023 13:39
@GeckoEidechse GeckoEidechse added the enhancement New feature or request label Aug 8, 2023
@Jan200101
Copy link
Contributor Author

Validated that the verison is now updated when changing the profile by switching to R2Ronin (which FlightCore cannot detect but is still a valid Profile)

@Jan200101
Copy link
Contributor Author

UI wise, there should maybe also be some explainer of what profiles are.

Future:tm:

Personally I'd still suggest to gate profile selecting behind DevView

Done

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

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

Confirmed working on Windows and Linux (skipped actually launching the game on Linux cause no working Titanfall2 install)

Requested code changes have been addressed and there isn't anything major I'd change with this PR rn. We could move the SKIP_PATHS and MAY_CONTAIN to constant.rs but I don't really see too much of a need to do so rn ^^

So good to merge from my end :D

@GeckoEidechse GeckoEidechse merged commit 24fb67f into R2NorthstarTools:main Aug 8, 2023
@Jan200101
Copy link
Contributor Author

based on #444

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants