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

Fix #3738 createBuilding optional arguments not handled correctly #3778

Closed
wants to merge 2 commits into from

Conversation

FileEX
Copy link
Contributor

@FileEX FileEX commented Oct 8, 2024

Fix #3738

I think this is what the parser should take care of, that if the vector is optional, it should put 0 in the remaining fields

@TheNormalnij
Copy link
Member

I don't think that original issue is valid. Developers should provide all vector arguments. It has more sense for me.

You fixes only one case of this issue. The main problem hides in argument parser.

@TracerDS
Copy link
Contributor

TracerDS commented Oct 8, 2024

I don't think that original issue is valid. Developers should provide all vector arguments. It has more sense for me.

You fixes only one case of this issue. The main problem hides in argument parser.

Its more of a logic issue. What do you expect vector to receive? 3 coordinates.
What happens when you dont provide all 3 coordinates? Error occurs.

@FileEX
Copy link
Contributor Author

FileEX commented Oct 8, 2024

TheNormalnij is right. This should be fixed in the parser. Many functions rely on the syntax where each argument in [ ] is optional, such as createVehicle, createObject, etc. The method in the old parser, ReadVector3D, would simply insert 0 in place of any missing argument. The new parser should handle this as well, because the current behavior is confusing. In one function, you can provide a single argument, but in another, you have to provide all three, even though each one is marked as optional individually

@TracerDS
Copy link
Contributor

TracerDS commented Oct 8, 2024

TheNormalnij is right. This should be fixed in the parser. Many functions rely on the syntax where each argument in [ ] is optional, such as createVehicle, createObject, etc. The method in the old parser, ReadVector3D, would simply insert 0 in place of any missing argument. The new parser should handle this as well, because the current behavior is confusing. In one function, you can provide a single argument, but in another, you have to provide all three, even though each one is marked as optional individually

for now lets just treat vectors as struct { float x,y,z; }

@Pirulax
Copy link
Contributor

Pirulax commented Oct 8, 2024

Should be an easy fix

TheNormalnij is right. This should be fixed in the parser. Many functions rely on the syntax where each argument in [ ] is optional, such as createVehicle, createObject, etc. The method in the old parser, ReadVector3D, would simply insert 0 in place of any missing argument. The new parser should handle this as well, because the current behavior is confusing. In one function, you can provide a single argument, but in another, you have to provide all three, even though each one is marked as optional individually

So, just default initialize values (CVector for example)?

@FileEX
Copy link
Contributor Author

FileEX commented Oct 8, 2024

Should be an easy fix

TheNormalnij is right. This should be fixed in the parser. Many functions rely on the syntax where each argument in [ ] is optional, such as createVehicle, createObject, etc. The method in the old parser, ReadVector3D, would simply insert 0 in place of any missing argument. The new parser should handle this as well, because the current behavior is confusing. In one function, you can provide a single argument, but in another, you have to provide all three, even though each one is marked as optional individually

So, just default initialize values (CVector for example)?

The point is that if a function accepts std::optional<CVector>, we have to provide the entire vector, all three values, because otherwise, there's an error that says a vector was expected, but a number was received. It should work like in the old parser, where if, for example, two values fX and fY were provided, the fZ value would simply be set to 0 and treated as a valid vector

@Pirulax
Copy link
Contributor

Pirulax commented Oct 8, 2024

yeah thats what i meant
that should be easy

@FileEX FileEX closed this Oct 8, 2024
@FileEX FileEX deleted the bugfix/building_args branch October 8, 2024 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createBuilding optional arguments not handled correctly
5 participants