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

some new code #81

Closed
wants to merge 18 commits into from
Closed

some new code #81

wants to merge 18 commits into from

Conversation

1Evolve
Copy link

@1Evolve 1Evolve commented Jan 26, 2024

Description

Rewrite the camera code
With the addition of new codes, and adjusting the code in a better way It is now easier to add new cameras to the code
Add a list of all cameras with numbers, names and addresses Add a tablet and animation when opening the camera andyy code

Checklist

  • I have personally loaded this code into an updated Qbox project and checked all of its functionality.
  • My pull request fits the contribution guidelines & code conventions.

Rewrite the camera code
With the addition of new codes, and adjusting the code in a better way
It is now easier to add new cameras to the code
Add a list of all cameras with numbers, names and addresses
Add a tablet and animation when opening the camera andyy code
for test isLeo(2, true)
@1Evolve 1Evolve changed the title rewrite camera code and switch to lib some new code Feb 15, 2024
@1Evolve 1Evolve requested a review from BerkieBb February 19, 2024 18:43
@BerkieBb
Copy link
Contributor

Check remaining comments

@1Evolve
Copy link
Author

1Evolve commented Mar 19, 2024

Is there any response to the request?

@1Evolve
Copy link
Author

1Evolve commented Mar 19, 2024

If it does not fit the idea of the project, please close the request

@1Evolve
Copy link
Author

1Evolve commented Mar 19, 2024

three months

Copy link
Contributor

@Manason Manason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the lint errors.

It looks like there are several new features and refactors in this PR. If you want faster reviews, consider breaking up your PRs into smaller bite sized chunks of one new feature, or one refactor. These large PRs can be difficult and time consuming to review with lots of back and forth comments.

if not checkLeoAndOnDuty(source) then return end
TriggerClientEvent('police:client:ActiveCamera', source, args.camid)
TriggerClientEvent('police:client:showCamera', source)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be better as a callback? I like to use events only when we expect/want multiple event handlers, but this seems to be more like a network function call use of events.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A callback is only used when you expect to receive something back, otherwise you use events. So this is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Callback inherently are also just events, so it won't make a difference if you do use it aside from you using more resources to do the same thing

end
return tostring(hours .. ':' .. minutes)

tabletProp = CreateObject(`prop_cs_tablet`, 0.0, 0.0, 0.0, true, true, false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be creating networked props on the client as it is not compliant with entity lockdown mode. Instead, create it on the server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@1Evolve just this one and you should be good

@1Evolve
Copy link
Author

1Evolve commented May 2, 2024

Completed 4 months ago
You can delete the pull request
I'm no longer as enthusiastic as I started out with

@mafewtm
Copy link
Member

mafewtm commented Jun 1, 2024

Closed as requested.

@mafewtm mafewtm closed this Jun 1, 2024
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.

4 participants