xpilot-project / xpilot

Open-source, cross-platform X-Plane pilot client for VATSIM.
https://xpilot-project.org
GNU General Public License v3.0
119 stars 36 forks source link

Allow to tune Unicom per click (plugin) #124

Closed jonaseberle closed 1 year ago

jonaseberle commented 1 year ago

This adds a new group "Unicom" with a row "Unicom 122.800" and a click spot to tune COM1 to the X-Plane plugin.

Relates: #121

xpilot-tune-unicom

I have not worked with ImGui before so I hope this is OK.

I am also not able to compile xPilot itself (not the plugin) so I cannot add that functionality to it.

jonaseberle commented 1 year ago

I have been flying around with it for the last days and am rather happy with it.

An alternative idea would be to get rid of the additional table row: image

Happy for feedback.

justinshannon commented 1 year ago

Great idea, however, I think it would be better to have a dedicated button for this action instead of a separate section. This would make it easier to find.

X-Plane_FvtfFY5iAI

jonaseberle commented 1 year ago

I think it would be more suited at the end of the active ATC list. Rationale: When in an ATC controlled area, really only the live controllers are important. Only after you looked through the active list you would want to tune Unicom.

I have not checked it in the sim but I think in the latest commit this is now shown always? I would only show it when connected. Rationale: It might give the false impression that you are now able to receive UNICOM messages.

I have put it into the list because the list is a list of frequencies so I found a different mental model is not necessary. Like this it falls apart usability-wise IMHO: The headset icon that has been learnt by users to mean "tune freq" is not used.

Also the green in that otherwise beautiful ImGui window does not look good IMHO.

But that's just me. I am happy to have that function anyways.

justinshannon commented 1 year ago

I agree with your assessment. I've adjusted it accordingly.

X-Plane_xyL2NIlbhg

jonaseberle commented 1 year ago

I've tested this together with https://github.com/xpilot-project/xpilot/commits/dev/plugin-tune-com2 (works great) and just needs the COM2 then, too.

Shall I rebase it onto https://github.com/xpilot-project/xpilot/commits/dev/plugin-tune-com2? It then just needs ImGui::SameLine(22) and the COM2 icon.

xpilot-lunicom-com2

Thank you!

justinshannon commented 1 year ago

This will be available in the next release.

jonaseberle commented 1 year ago

I haven't really paid attention when that happened. You did not merge this but authored a new commit https://github.com/xpilot-project/xpilot/commit/a77597a86f0184cf5216c01324da2f9e2eaaf5a7 keeping me completely out of it. That is a bit unusual and leaves a slightly sore feeling.

Well, to be totally honest, I'd like to be listed as contributor.

justinshannon commented 1 year ago

Sorry, Jonas, that wasn’t my intention. I may have add a contributor section to the README since merging didn’t do anything.

justinshannon commented 1 year ago

@jonaseberle

Actually, it looks like you’re listed as a contributor now. Can you confirm?

jonaseberle commented 1 year ago

Yes, thanks. Sorry if I was skeptical there. All good.

leecollings-sb commented 10 months ago

Just wanted to ask a question on this (considering it's already been released). Is there a way to add a button or ability to switch either COM1 or COM2 directly to unicom within the application UI?

The logical place would be right above 'Nearby ATC'.