Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adding Direct Vehicle Move waypoint #11
base: master
Are you sure you want to change the base?
Adding Direct Vehicle Move waypoint #11
Changes from 4 commits
2faccd2
3486c18
649e0ca
59b185e
d98bcda
376c288
73174e6
f57b917
20cd57d
de610a6
1cdccc0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Vehicle Move
orMove Vehicle
, stringtable naming is inconsistent with class. Also usecamelpascal case for this (VehicleMove or MoveVehicle)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.
Actually
VehicleMove
orMoveVehicle
would bePascalCase
, notcamelCase
.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.
You should keep one empty line before and after any list in Markdown. Plus some spelling fixes.
I'm not sure what that does mean, so if my suggestion doesn't apply, please fix that.
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.
Comment does not follow the template, excess line between author and arguments.
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.
Unnecessary comment, it's obvious what
_vehicles = [];
means.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.
You should save and restore values of these variables after the waypoint is completed.
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.
What if next waypoint is the same? Will there be 2 competing scripts? I don't see a valid reason for checking
waypointScript
. The only thing that should matter is active waypoint number (you could pass it as an argument to WUAE to have the old value for comparison).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.
but waypoint gonna end only when WUAE is complete, so new wuae gonna be created for next waypoint
and this is to complete WUAE after ZEUS change Waypointype
edit. In arma 2.18 gonna be a option to replace it by passing _ThisScript to WUAE
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.
Relevant FT ticket: https://feedback.bistudio.com/T183544
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.
Nice, can be resolved then.
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.
What does this achieve? Does this reset pathfinding?
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.
Trying reset driver mostly in situationn where driver move vehicle super slowly after hitting object or moving super slowly in turning to waypoint
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.
Please review your whole code for such cases, there's plenty of them. Please stick to camelCase/the same casing as on Biki.
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.
Please keep a space after
,
. Also multiple occurrences.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 see veteran spared his time writing such comments and will have you fix all the issues that will be automatically reported by updated HEMTT. 🤣
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.
Personally I think plural would be more in-place for component concerning (possibly) multiple waypoints.
Keep in mind renaming folder.
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 agree, do not forget to change pboprefix and folder names too.