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 Shuttle Crash Crash For Hijack #3674

Merged
merged 1 commit into from
Jun 22, 2023

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Jun 19, 2023

This PR fixes a crash... with shuttles crashing! Namely with this runtime on round 17377 that prevent the shuttle from launching:

[2023-06-19 11:33:51.878] runtime error: Cannot read null.related
 - proc name: get area turfs (/proc/get_area_turfs)
 -   source file: code/__HELPERS/unsorted.dm,1251
 -   usr: Young Queen (/mob/living/carbon/xenomorph/queen)
 -   src: null
 -   usr.loc: the floor (35,50,2) (/turf/open/shuttle/dropship)
 -   call stack:
 - get area turfs(/area/almayer/shipboard/brig/d... (/area/almayer/shipboard/brig/dress))
 - /datum/dropship_hijack/almayer (/datum/dropship_hijack/almayer): target crash site("Upper deck Foreship")
 - the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight): hijack(Young Queen (/mob/living/carbon/xenomorph/queen), 0)
 - the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight): attack alien(Young Queen (/mob/living/carbon/xenomorph/queen))
 - Young Queen (/mob/living/carbon/xenomorph/queen): UnarmedAttack(the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight), 1, /list (/list), 0, 0)
 - Young Queen (/mob/living/carbon/xenomorph/queen): click adjacent(the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight), null, /list (/list))
 - Young Queen (/mob/living/carbon/xenomorph/queen): do click(the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight), the floor (36,51,2) (/turf/open/shuttle/dropship), "icon-x=24;icon-y=6;left=1;butt...")
 - SOMEONE (/client): Click(the dropship navigation comput... (/obj/structure/machinery/computer/shuttle/dropship/flight), the floor (36,51,2) (/turf/open/shuttle/dropship), "mapwindow.map", "icon-x=24;icon-y=6;left=1;butt...")

Explain why it's good for the game

Not only does this fix the case of a non-existent /area/almayer/shipboard/brig/dress being selected, it will now evenly weight the possibilities of landing sites. Previously this code would favor smaller areas because an area that was 1 tile would be just as likely as an area with 20k tiles.

Testing Photographs and Procedure

Screenshots & Videos

image

Changelog

🆑 Drathek Firartix
fix: Fixed a crash with hijack code possibly picking a non-existent brig area, and weighting smaller areas heavier.
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Jun 19, 2023
Copy link
Member

@morrowwolf morrowwolf left a comment

Choose a reason for hiding this comment

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

Code looks good let's give it a few rounds on live 🙏

@fira
Copy link
Member

fira commented Jun 20, 2023

wouldn't it be simpler to put a var tag on the areas than having the list hardcoded in?

@Drulikar
Copy link
Contributor Author

wouldn't it be simpler to put a var tag on the areas than having the list hardcoded in?

I don't know about simpler, that just fragments up the logic (as in every line that is here would instead be a line setting a value true in various files that define these zones). Its also not something that should be generically added to abstract areas (e.g. a boolean set true in say /area/almayer/shipboard/brig to affect all child areas of the brig) either unless you know for sure that area doesn't touch the exterior of the ship so that you know the bay doors don't open to space. Now I would need to search for all references of that tag and modify its value in various files. By all being in one place, you know all the possibilities when crashing the upper aft of the ship.

Its also not like this needs to be performant either. This proc will be used 0-2 times per round. The only argument I can really see for that is better future proofing for more ship maps (assuming its done is a straight forward enough way that all future crash landable areas would get set up from the beginning); but that is not even anything anyone is currently pursuing doing.

@morrowwolf morrowwolf added this pull request to the merge queue Jun 22, 2023
Merged via the queue into cmss13-devs:master with commit 608efdb Jun 22, 2023
github-actions bot added a commit that referenced this pull request Jun 22, 2023
cm13-github added a commit that referenced this pull request Jun 22, 2023
@Drulikar Drulikar deleted the Fix_Shuttle_Crashing branch June 22, 2023 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants