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
upgrade working for 5.1 #1051
upgrade working for 5.1 #1051
Conversation
Thanks for adding improvements/changes. The plugin is currently compatible with all versions UE5.0 to 5.3, using Windows and Visual Studio. I'm adding comments/reviews to remove references to UE5.1, which break compatibility with UE5.0. Also it would be good to update the Unreal plugin Readme so that people are aware of the buildplugin.bat and how to use it. |
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.
Keep reference to 5.0
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.
Keep reference to 5.0
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.
Keep reference to 5.0
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.
If wanting to update plugin version number, I'd suggest version 1.02
"Version": 102,
"VersionName": "1.02",
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.
It would be good to keep no engine version reference if possible, or at least use 5.0 as minimum (if possible).
Also update the Unreal Readme to make note this new bat file exists 😃 and it's usage.
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.
These are automatic changes from UE5, no need to include changes.
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.
These are automatic changes from UE5, no need to include changes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1051 +/- ##
=======================================
Coverage 24.97% 24.97%
=======================================
Files 168 168
Lines 18057 18057
=======================================
Hits 4510 4510
Misses 13547 13547 ☔ View full report in Codecov by Sentry. |
@gallonmate, @tsky1971 is this PR ready for merging ? Or are there still some pending issues ? |
No this PR is not ready. I'm not even 100% sure this PR is necessary in the first place BUT I saw no harm as long as the author resolved the compatibility for all UE5 versions, which they did not address yet, (This PR breaks compatibility with UE5.0). If you don't hear anything from the author, feel free to close the PR. |
My 2 cents here. First, thanks for the great work. Second.
There were major changes between 5.0 and 5.1 as I read... With some very naughty fixes partially based on this PR I managed to compile and run jsbsim on 5.3.2 linux (popos). (Half time startup crashes, but i need to get more acquainted with unreal - it is a new stuff for me ;) ) So.. If it possible please do not remove this PR. As unfinished as it is, it is still useful for a newcomers like me :) Bests. |
@lgzajac When closing a PR, it's just marked as closed. It can still be found when searched via keywords. The PR is not actually removed. Also there is no major changes between UE5.1 and UE5.0 that required breaking the plugin's compatibility. Keeping compatibility is easy but the author never finished addressing the reviews/comments from 2 months ago. Closing the PR for now allows the JSBSim repo maintainers to keep clarity and focus. UE5.1 was released in Nov 2022 and there are been enough users without complaint, including the original Epic employee who created the plugin, that I do no see any concern. Although I tested the new UE5.4 and it might actually need small fixes similar to this PR. Since the author is busy/missing then closing this PR and creating a new PR is the most likely outcome. |
Closing this PR following @gallonmate's suggestion. As a side note, if the information contained in this pull request is valuable to the community, it may be beneficial to contribute a new article to our wiki rather than expecting non-developers to delve into the PRs. |
fix iwyu (https://docs.unrealengine.com/5.0/en-US/include-what-you-use-iwyu-for-unreal-engine-programming/)
fix/add missing category for blueprint (needs for 5.1)
compile with 5.1
add build batch (buildpath is more temp - long name of the plugin leads easy to max path length >254)