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

Update to new .getCategory #247

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Update to new .getCategory #247

merged 1 commit into from
Dec 12, 2023

Conversation

Penecruz
Copy link
Contributor

@Penecruz Penecruz commented Nov 18, 2023

Update the code to be compatible with the new .getCategory() implementation which ED changed in the latest release.

@rurounijones rurounijones changed the title Fix Bug with .getCategory Update to new .getCategory Update the code to use the new .getCategory implementation whicH ED changed in the latest release. Nov 19, 2023
@rurounijones rurounijones changed the title Update to new .getCategory Update the code to use the new .getCategory implementation whicH ED changed in the latest release. Update to new .getCategory Nov 19, 2023
@peterb154
Copy link

Note comment in issue #246 (comment)

@martinco
Copy link
Contributor

I've also been running with the same set of changes as this PR, with the additional change to the reference in lua/DCS-gRPC/methods/controllers.lua updated.

I haven't had any issues since, no errors, or stream stopping (I left only the group:getCategory() in exporters/object.lua for group as i haven't had a chance to test it but with 900 units, and regular destruction and player slot changes on a multiplayer server, no issues since making this set of changes

@mobot-gh
Copy link
Contributor

I haven't had any issues since, no errors, or stream stopping (I left only the group:getCategory() in exporters/object.lua for group as i haven't had a chance to test it but with 900 units, and regular destruction and player slot changes on a multiplayer server, no issues since making this set of changes

If the reference in exporters/object.lua is left as is, the return values will be from the Group.Category enum and not the Object.Category enum, right? If so, any client code with conditionals based on that value would need to be tweaked unless the line is changed, like the others, to Object.getCategory(group).

@martinco
Copy link
Contributor

Sorry for the confusion, all I meant by that anecdote was more the other unit known case and that seems good

But in the group case I haven't checked the behavior/ what it should be and would be guesswork on my part to change without verification/cause so pointed it out but haven't acted on it

Apologies for the anecdote; its more that it's not impacting me doesn't mean it's not a problem

@mobot-gh
Copy link
Contributor

mobot-gh commented Nov 20, 2023

Sorry for the confusion, all I meant by that anecdote was more the other unit known case and that seems good

But in the group case I haven't checked the behavior/ what it should be and would be guesswork on my part to change without verification/cause so pointed it out but haven't acted on it

Apologies for the anecdote; its more that it's not impacting me doesn't mean it's not a problem

No need to apologise, I added my comment as more of a question to see if it is only units that are affected by the change to .getCategory() or if Weapons, Groups, and Statics are also included. Maybe mygroup:getCategory() was already returning values from the Group.Category enum? Only some more testing will help us understand unless someone knows for certain already.

[EDIT/UPDATE]: group:getCategory() should work as before without needing any change. Source: ED Discord

@Penecruz
Copy link
Contributor Author

Uncertain of other impacts, the changes in this PR have allowed OverlordBot to function without issue, which is my only application using DCS-gRPC. My understanding was only Object was returning incorrect data prior to the change ED made in latest OB.

@rurounijones
Copy link
Contributor

rurounijones commented Dec 6, 2023

Can you rebase this on to the current main branch which should fix lint failures.

Also add an entry to CHANGELOG.md

@mobot-gh
Copy link
Contributor

mobot-gh commented Dec 6, 2023

There is also the reference mentioned by @martinco on L25 of controllers.lua. Should that be included in this PR?

- Fix for DCS API change to getCategory()
@Penecruz
Copy link
Contributor Author

There is also the reference mentioned by @martinco on L25 of controllers.lua. Should that be included in this PR?

Not needed in this case.

@rurounijones rurounijones merged commit 4f4b8bc into DCS-gRPC:main Dec 12, 2023
4 checks passed
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.

5 participants