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

Add better multiple hospital support + some refactors #299

Merged
merged 12 commits into from
Mar 18, 2024

Conversation

BackSH00TER
Copy link
Contributor

Describe Pull request
The main purpose of this PR is to tweak the existing implementations to make it easier to add support for multiple hospitals and clean up the logic surrounding this. It also cleans up a lot of the logic with regards to finding a free bed at the hospital.

  • Updates Config.Locations to add each hospital location and its corresponding bed locations
  • Combines GetAvailableBed and SetClosestBed with a newer function getClosestAvailableBed that takes the index of the hospital you are at to reduce the number of loops that are needed.
  • Uses hospitalIndex to select the correct bed at that hospital location
  • Updated areas that were previously looking at Config.Locations.beds to now look at Config.Locations.hospital[hospitalIndex].beds
  • Updated some strings to use the correct hospital name

Tested:

  • Hold E to respawn will still respawn you at Pillbox
  • Checkin at hospitals will put you in the first open bed at that hospital
  • Tested getting on hospital bed by walking up to and using the nearby functions to sit on bed
  • Tested that beds isTaken status correctly resets when you leave the bed

Example showing checkin + sit on bed
https://clipchamp.com/watch/DL7ruUxMQpD

@nzkfc
Copy link
Contributor

nzkfc commented May 28, 2023

@BackSH00TER Could you please add it so it will check which hospital you're closest too and spawn you there instead of pillbox?

Also I noticed when you set checkin locations, you have to also ensure the "Location" for each hospital is the same vector? otherwise it won't let you checkin?

@github-actions
Copy link

This PR has had 60 days of inactivity & will close within 7 days

@github-actions github-actions bot added the Stale label Sep 24, 2023
@github-actions github-actions bot removed the Stale label Sep 25, 2023
Copy link

This PR has had 60 days of inactivity & will close within 7 days

@github-actions github-actions bot added the Stale label Dec 22, 2023
@BackSH00TER
Copy link
Contributor Author

Commenting to remove staleness. Can someone from QB team comment whether or not this is something you would want to merge in? If yes, I'll update and fix the merge conflicts, if no, then lets just close this out

@github-actions github-actions bot removed the Stale label Dec 23, 2023
@Twikii
Copy link

Twikii commented Dec 26, 2023

Commenting to remove staleness. Can someone from QB team comment whether or not this is something you would want to merge in? If yes, I'll update and fix the merge conflicts, if no, then lets just close this out

I have try it on my server, its working smoothly, thanks bro!

@Irishstevie
Copy link

I can say this is something that would be awesome to see

Copy link

@TheBlokker TheBlokker left a comment

Choose a reason for hiding this comment

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

Perfect i dont see any problem

@BackSH00TER
Copy link
Contributor Author

Commenting to remove staleness. Can someone from QB team comment whether or not this is something you would want to merge in? If yes, I'll update and fix the merge conflicts, if no, then lets just close this out

@GhzGarage I would appreciate if you could take a look at this if you have a chance. I'm more than happy to update and fix the merge conflicts if this is something you would actually want merged.

@nzkfc
Copy link
Contributor

nzkfc commented Jan 22, 2024 via email

Copy link

@TheBlokker TheBlokker left a comment

Choose a reason for hiding this comment

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

I checked it again, and not error, I see no problem in the code.

Is a error in testing?

@BackSH00TER
Copy link
Contributor Author

I'd say do the updates anyways man, did you add the hospital distance checking too determine where too spawn?

I'm not going to waste my time continually updating and fixing merge conflicts just for the PR to sit unacknowledged for another 7+ months.

If I get the green light from somebody who is actually on the QB team and has merge permissions, then I will be more than happy to update it.

@nzkfc
Copy link
Contributor

nzkfc commented Jan 23, 2024 via email

@c-drive
Copy link

c-drive commented Feb 11, 2024

+1
This commit should really be added to master, it fixes a litany of issues present in the current code.

@jaylac2000
Copy link

PLZ Merge !!

@SinisterDevelopment
Copy link

i have pushed this to be merged!

Copy link

@TheBlokker TheBlokker left a comment

Choose a reason for hiding this comment

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

ready to be merged

@GhzGarage
Copy link
Member

@SinisterDevelopment See if you can make a new PR on this without conflicts

@jaylac2000
Copy link

@GhzGarage

i might have a copy of it without conflicts. just gotta delete my m-Insurance integration and reset config if you want it

@BackSH00TER
Copy link
Contributor Author

BackSH00TER commented Mar 1, 2024

@SinisterDevelopment See if you can make a new PR on this without conflicts

@GhzGarage I can update this tonight

@BackSH00TER
Copy link
Contributor Author

Merge conflicts are fixed. This is ready to go again

@BackSH00TER
Copy link
Contributor Author

@GhzGarage @SinisterDevelopment this PR is ready to go. Please approve so this PR can be merged. Thank you.

@GhzGarage GhzGarage merged commit 17e844a into qbcore-framework:main Mar 18, 2024
2 checks passed
@BackSH00TER BackSH00TER deleted the multiple-hospital-support branch March 18, 2024 23:34
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.

9 participants