-
Notifications
You must be signed in to change notification settings - Fork 285
Clarify Enable Desktop Support checkbox for all project types #3148
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
base: main
Are you sure you want to change the base?
Clarify Enable Desktop Support checkbox for all project types #3148
Conversation
- Java: Has no effect, can be checked or unchecked - C++: Must be checked for simulation, unit tests, Romi/XRP projects - Added note about third-party library compatibility - Removed Python reference (doesn't use this project creator) Fixes wpilibsuite#3146
|
Does it need to be checked for C++ Romi and XRP projects? Since they use different templates, I think it might be ignored. |
|
I am not positive here but I thought you had to have it for those and that was what the training was asking you to do from the original issue? |
|
Well the original issue said they didn't know if they needed to select enable desktop support, but suspected it did need to be checked. I'm just saying we should run the test before writing the documentation. I tend to suspect it's ignored, but just like the original issue writer, I'm not sure. |
|
Yeah, the C++ Romi/XRP templates don't have an |
|
So it really sounds like that portion of the video needs to be removed |
What video? |
|
I was referring to the Java Programming Part 1 video that triggered the original issue. On reviewing them the Romi project erroneously instructs the user to check the box while the XRP project doesn't mention it and leaves it unchecked. Probably not a big deal since it doesn't matter. |
Per Gold856's comment, the C++ Romi/XRP templates don't have an includeDesktopSupport variable in the build.gradle, so the Enable Desktop Support checkbox is a no-op for those templates. Updated documentation to explicitly state that the checkbox has no effect for Romi and XRP templates and can be left checked or unchecked. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
| - **C++**: Checking this option enables desktop compilation, which is required for simulation and unit tests | ||
| - **Romi/XRP**: This option has no effect for Romi and XRP templates and can be left checked or unchecked | ||
|
|
||
| .. note:: While WPILib fully supports desktop builds, some third-party vendor libraries may not. If a library doesn't support desktop compilation, your C++ code may not compile or may crash when running simulation. In such cases, you may need to conditionally compile code that uses those libraries (see :doc:`/docs/software/wpilib-tools/robot-simulation/introduction`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the link should probably go on the line with enable desktop support. Right now it implies it tells you how to conditionally compile code, but that link doesn't address that.
Per sciencewhiz's comment, moved the robot simulation introduction link from the note to the main "Enable Desktop Support" line. The link is a general reference about simulation, not specifically about conditional compilation, so it was misleading to have it in the note that discusses vendor library compatibility issues. Also removed the conditional compilation reference from the note since the linked documentation doesn't address that topic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Improves the documentation for the "Enable Desktop Support" checkbox in the New Project Creator, addressing confusion reported in issue #3146. This fix applies to all project types, not just XRP.
Changes
Replaced the vague existing text with clear guidance:
Before:
After:
Why This Matters
Users (especially those following tutorials) were confused about whether to check this box. The original text suggested it should be "left unchecked unless needed," but didn't explain that:
Additional Changes
Fixes #3146