Refactors secbot code, cleans up bot code in general. Centralizes defines #8492

Tsar-Salat · 2023-02-13T17:01:31Z

Ports:

makes the ed209 a child of securitrons instead of a horrifying copypaste tgstation/tgstation#46230
Beepsky refactor and Bot code improvement tgstation/tgstation#62510

About The Pull Request

Okay, so right now the ED-209 is basically just copy-paste of securitron code, which is really unecessary in the grand scheme of things. This makes ED-209s a child of securitrons, so you can just call /secbot/ instead of both.

Some other stuff:

Removed lasertag stuff from ED-209 code, because it was unused in-game.
(Probably) ED-209 no longer has shitty aim, and can actually shoot you when you lay down, as intended (police brutality)
misc fixes

PR 2

Refactors declare_arrests, idcheck, weaponscheck, check_records, arrest_type into flags
Makes Armsky a subtype of Beepsky rather than being varedited in every map
Re-organizes Robot defines to make up for this
Generally improves the code with early returns
Splits ED-209s into their own bot type rather than have a complete var decided to just saying if its a ED-209 or not
cleans up baton_type and weapon vars

Additionally:

cleans up a lot of explode and item drops from bots, especially secbots and especially especially Grievous.
Removes nullcrate variant of Grievous, as nullcrates dont exist
Ensures Grievous & Toy Grievous follow their weapon damages

Why It's Good For The Game

Less shitcode is goodcode.

Following this PR will be a floorbot cleanup, then we can finally TGUI these fucking things

Testing Photographs and Procedure

Screenshots&Videos

Grievous works.

dreamseeker_WN3ETaooeg.mp4

All secbots properly share the same behavior and take me out. Sergeant-At-Armsky starts on the Detain setting, so he babysits me.

dreamseeker_CQxKzUaf5K.mp4

Changelog

🆑 RKz, Fikou, JohnFulpWillard
remove: ED-209 lasertag functionality (it was never implemented and no one was ever gonna make one if I made it functional IC)
fix: ED-209 aim isn't as broken, it shouldn't be harder to shoot a stationary target laying down when it can hit a moving target running down the hall
fix: fixed a few misc errors in ED-209 code
refactor: ED-209s are children of secbots now. Beepsky finally got his kids from the divorce.
code: Centralizes all bot defines.
refactor: Turns Sergeant-At-Armsky into his own subtype, rather than a mapedit on every map...
refactor: refactors most bot checks into flags
remove: Removes nullcrate variant of Secbot Grievous, as nullcrates havent existed in over two years...
/:cl:

code/datums/components/crafting/recipes.dm

Rukofamicom · 2023-02-13T19:13:05Z

someone spelled "Judgement" wrong, freakin goobers smh
spellcheck: It's spelled "Judgement"!

Check yourself.
Judgment does not have an E.

Tsar-Salat · 2023-02-13T19:26:32Z

Judgment does not have an E.

Helltaker uses "Judgement" so clearly thats the correct frame of reference here. 💯

Rukofamicom · 2023-02-13T19:44:46Z

Please do not introduce spelling mistakes into code just because someone in a place outside of Bee made the same spelling mistake.

Fix your PR

Tsar-Salat · 2023-02-13T20:11:39Z

Please do not introduce spelling mistakes into code just because someone in a place outside of Bee made the same spelling mistake.

This is such bullshit. It literally exists in other parts of the code as "judgement".

All I did was change one use case and you are up in arms because you didn't do your homework and check the spelling yourself.

Can you stop pontificating already?

Rukofamicom · 2023-02-13T20:35:46Z

Making an issue worse is not justification for making the change.

But yes I will stop here. It's something for maints to decide whether it is a frivolous and unnecessary change, not me ultimately

Fronsis · 2023-02-14T08:38:15Z

Since this is ED related.. could i request an update on the recipe on the "crafting menu" section of EDs? Because a lot of new(and some veterans) Roboticists guide themselves by the items required on crafting menu and they go "uhh we need dragnets" or just fail to make them because the recipe is outdated, we currently use the building method shown in the Wiki of Roboticists(and i prefer that one tbh)

Tsar-Salat · 2023-02-14T16:38:26Z

Roboticists guide themselves by the items required on crafting menu and they go "uhh we need dragnets" or just fail to make them because the recipe is outdated, we currently use the building method shown in the Wiki of Roboticists(and i prefer that one tbh)

Yes, that is fixed in this PR.

Okay, so for building stuff, thing are separated into recipes (seen in crafting menu), and build steps (like adding a cyborg arm to a medkit will make it an unfinished medborg)

Basically, someone changed the building steps of making ED-209s to disabler, but didn't change the recipe. This didn't make sense to me so I changed it both to disabler, which was what Ivan was calling an "undocumented change" above.

I saw the difference when I was gutting laser tag beams from the ed209.dmm, and just went ahead and made them congruent

PowerfulBacon

Even though I agree that the Americans are wrong and couldn't spell hard words, changing variable names and proc names is frowned upon with no good reason either way, since it makes for hassles regarding conflicts and porting. If it's in the code, there is no reason to correct spelling mistakes really, as long as the name is sensible and the point is given.

Tsar-Salat · 2023-02-16T18:12:31Z

If it's in the code, there is no reason to correct spelling mistakes really, as long as the name is sensible and the point is given.

Done

Addressed

zeskorion · 2023-02-20T10:25:44Z

someone spelled "Judgement" wrong, freakin goobers smh
spellcheck: It's spelled "Judgement"!

Check yourself. Judgment does not have an E.

judgement does in fact have an E. are you using incorrect (british) spelling?

||(yes, I am aware that judgement is actually the british spelling of the word)||

itsmeow · 2023-03-07T21:27:20Z

did you get ED-209 sprite fixed?

github-actions · 2023-03-11T03:09:51Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Rukofamicom · 2023-10-24T17:29:58Z

Remove the snowflake code that you added for bluespace cells, it should accept all types of power cells for the sake of consistency.

You can add code that adjusts the bot based on cell type, but you can't blacklist a single type of cell just because.

Update PR body and CL as well.

Tsar-Salat · 2023-10-24T17:51:06Z

but you can't blacklist a single type of cell just because.

Yeah, I did it just because. My thoughts exactly.

code/modules/mob/living/simple_animal/bot/ed209bot.dm

Rukofamicom · 2023-10-25T11:41:05Z

code/modules/mob/living/simple_animal/bot/ed209bot.dm

-	var/cell_type = /obj/item/stock_parts/cell
-	var/vest_type = /obj/item/clothing/suit/armor/vest


Why did these just get removed instead of storing what was used to make the bot? The components used in construction should be stored so they can be dropped via drop_part() when the bot is destroyed.

I'd recommend storing them in a list rather than individual variables though if you were trying to keep it clean, I touch more on this later in the explode() proc review.

handled for now. Can confirm they drop during /explode()

This is not handled, the variables are still removed instead of keeping the items stored for dropping later.

Again, a list is the easiest way to do this.

code/modules/mob/living/simple_animal/bot/secbot.dm

github-actions · 2023-10-26T18:01:34Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Tsar-Salat · 2023-10-26T18:07:26Z

Damn

Tsar-Salat · 2023-10-26T22:07:57Z

Im gonna remove those weirdass nullcrate-specific Grievous' too, since for some reason they were left in after Nullcrates were removed

Rukofamicom · 2023-10-27T01:41:42Z

Has this been thoroughly re-tested since adding another port on top?

First thing that caught my eye while skimming was this:

There are two step nines now.

Tsar-Salat · 2023-10-27T05:21:51Z

Has this been thoroughly re-tested since adding another port on top?

It's in testing evidence. Updated when I marked the PR as ready.

There are two step nines now.

Of which the number for the define ASSEMBLY_NINTH_STEP is 8. It is in the correct order.

Rukofamicom

This is all I can get to right now, but I want to clarify what I'm asking for probably can't be resolved by porting. You're going to have to adapt the port sometimes instead of doing it 1:1

Adding the second port on top only made this more complicated without addressing two of the three things I requested.

Rukofamicom · 2023-10-27T11:21:10Z

code/__DEFINES/robots.dm

+#define ASSEMBLY_SIXTH_STEP 5
+#define ASSEMBLY_SEVENTH_STEP 6
+#define ASSEMBLY_EIGHTH_STEP 7
+#define ASSEMBLY_NINTH_STEP 8


Add another one and replace the `if(9) with it.

Rukofamicom · 2023-10-27T11:23:23Z

code/__DEFINES/robots.dm

+///Alert when a new Cyborg is created.
+#define NEW_BORG 1
+///Alert when a Cyborg selects a model.
+#define NEW_MODULE 2
+///Alert when a Cyborg changes their name.
+#define RENAME 3
+///Alert when an AI disconnects themselves from their shell.
+#define AI_SHELL 4
+///Alert when a Cyborg gets disconnected from their AI.
+#define DISCONNECT 5


Aren't these deprecated now?

Rukofamicom · 2023-10-27T11:35:10Z

code/modules/mob/living/simple_animal/bot/SuperBeepsky.dm

+	var/judgement_criteria = judgement_criteria()
 	for (var/mob/living/carbon/C in view(7,src)) //Let's find us a criminal
 		if((C.stat) || (C.handcuffed))
 			continue

 		if((C.name == oldtarget_name) && (world.time < last_found + 100))
 			continue

-		threatlevel = C.assess_threat(judgment_criteria, weaponcheck=CALLBACK(src, PROC_REF(check_for_weapons)))
+		threatlevel = C.assess_threat(judgement_criteria, weaponcheck=CALLBACK(src, PROC_REF(check_for_weapons)))


Do I really need to dig up the last conversation about trying to "fix" the spelling of words that are different between british and american English?

Rukofamicom · 2023-10-27T11:48:15Z

code/modules/mob/living/simple_animal/bot/ed209bot.dm

-	var/cell_type = /obj/item/stock_parts/cell
-	var/vest_type = /obj/item/clothing/suit/armor/vest


This is not handled, the variables are still removed instead of keeping the items stored for dropping later.

Again, a list is the easiest way to do this.

Rukofamicom · 2023-10-27T11:54:48Z

code/modules/mob/living/simple_animal/bot/secbot.dm

+	if(bot_type == ADVANCED_SEC_BOT)
+		var/obj/item/bot_assembly/ed209/ed_assembly = new(Tsec)
+		ed_assembly.build_step = ASSEMBLY_FIRST_STEP
+		ed_assembly.add_overlay("hs_hole")
+		ed_assembly.created_name = name
+		new /obj/item/assembly/prox_sensor(Tsec)
+		var/obj/item/gun/energy/disabler/disabler_gun = new(Tsec)
+		disabler_gun.cell.charge = 0
+		disabler_gun.update_appearance()
+		if(prob(50))
+			new /obj/item/bodypart/l_leg/robot(Tsec)
+			if(prob(25))
+				new /obj/item/bodypart/r_leg/robot(Tsec)
+		if(prob(25))//50% chance for a helmet OR vest
+			if(prob(50))
+				new /obj/item/clothing/head/helmet(Tsec)
+			else
+				new /obj/item/clothing/suit/armor/vest(Tsec)
+	else
+		var/obj/item/bot_assembly/secbot/secbot_assembly = new(Tsec)
+		secbot_assembly.build_step = ASSEMBLY_FIRST_STEP
+		secbot_assembly.add_overlay("hs_hole")
+		secbot_assembly.created_name = name
+		new /obj/item/assembly/prox_sensor(Tsec)


Right now this proc is exactly like the code you're aiming to fix with this PR, and I question why code very specific to ED209, not potential future ranged bots, is being defined in the parent type.

If component parts are stored in a list, most of this proc could be simplified to this to make it drop an assortment of random parts

for(var/obj/item/I in component_parts) if(prob(50)) drop_part(I, loc)

And this also future-proofs it against later bots.

Alright, I think ive cleaned it to a reasonable degree that the ranged var is no longer an issue.

Er.. no this looks almost exactly like it did before, the ranged variable wasn't the issue and you're still effectively using it with a define anyway. You're hard-spawning items that may have nothing to do with what was used in the construction of the ED209. Use a list or use the variables you removed at bare minimum.

Might be an idea to have a unit test which constructs an EDD from a non-standard vest type, explodes it and then confirms that the correct construction types are being dropped.

the fuzz

751dc6d

solid-octo-guacamole bot added Code Improvement Fix Grammar and Formatting Refactor Tweak labels Feb 13, 2023

ivanmixo reviewed Feb 13, 2023

View reviewed changes

code/datums/components/crafting/recipes.dm Show resolved Hide resolved

PowerfulBacon previously requested changes Feb 16, 2023

View reviewed changes

requested

9fd7692

itsmeow added DO NOT MERGE Needs Testing Evidence labels Mar 7, 2023

github-actions bot added the Merge Conflict label Mar 11, 2023

Merge remote-tracking branch 'Upstream/master' into ED-shitcode-fix

d34dc8a

github-actions bot removed the Merge Conflict label Mar 13, 2023

oops proc

8d36fd6

Tsar-Salat marked this pull request as draft March 16, 2023 12:46

Tsar-Salat closed this Mar 16, 2023

Tsar-Salat mentioned this pull request Jun 5, 2023

Minor spelling mistake #9176

Merged

.

01d3e41

Rukofamicom requested changes Oct 25, 2023

View reviewed changes

https://github.com/tgstation/tgstation/pull/62510

9b17ba2

github-actions bot added the Administration label Oct 25, 2023

map changes

c5ca606

Tsar-Salat requested a review from PestoVerde322 as a code owner October 25, 2023 13:34

github-actions bot added the Mapping DMM Change label Oct 25, 2023

Tsar-Salat marked this pull request as draft October 26, 2023 03:15

github-actions bot added the Merge Conflict label Oct 26, 2023

Tsar-Salat added 4 commits October 26, 2023 15:57

cleanup

e9eda30

clean up drop_part

e75ff3a

cleanup defines

6200539

cleanup defines

e423687

Tsar-Salat changed the title ~~Kills awful ed209 copy-paste shitcode~~ Refactors secbot code, cleans up bot code in general. Centralizes defines Oct 26, 2023

Merge remote-tracking branch 'upstream/master' into ED-shitcode-fix

53c4703

github-actions bot removed the Merge Conflict label Oct 26, 2023

fixes bad init on grievous secbots

fb8a06d

Tsar-Salat added 3 commits October 26, 2023 18:23

remove grievous nullcrate bot, fix grievous loot

b4d72ca

fixes toy grievous

cbea39b

kilo-specific warden armsky

aba9e05

Tsar-Salat marked this pull request as ready for review October 26, 2023 23:06

Rukofamicom requested changes Oct 27, 2023

View reviewed changes

Tsar-Salat closed this Oct 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactors secbot code, cleans up bot code in general. Centralizes defines #8492

Refactors secbot code, cleans up bot code in general. Centralizes defines #8492

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Rukofamicom commented Feb 13, 2023

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Rukofamicom commented Feb 13, 2023

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Rukofamicom commented Feb 13, 2023 •

edited

Loading

Fronsis commented Feb 14, 2023

Tsar-Salat commented Feb 14, 2023 •

edited

Loading

PowerfulBacon left a comment

Tsar-Salat commented Feb 16, 2023

zeskorion commented Feb 20, 2023 •

edited

Loading

itsmeow commented Mar 7, 2023

github-actions bot commented Mar 11, 2023

Rukofamicom commented Oct 24, 2023 •

edited

Loading

Tsar-Salat commented Oct 24, 2023

Rukofamicom Oct 25, 2023

Tsar-Salat Oct 26, 2023

Rukofamicom Oct 27, 2023

github-actions bot commented Oct 26, 2023

Tsar-Salat commented Oct 26, 2023

Tsar-Salat commented Oct 26, 2023

Rukofamicom commented Oct 27, 2023

Tsar-Salat commented Oct 27, 2023

Rukofamicom left a comment

Rukofamicom Oct 27, 2023

Rukofamicom Oct 27, 2023

Rukofamicom Oct 27, 2023

Rukofamicom Oct 27, 2023

Rukofamicom Oct 27, 2023 •

edited

Loading

PowerfulBacon Oct 27, 2023

		var/cell_type = /obj/item/stock_parts/cell
		var/vest_type = /obj/item/clothing/suit/armor/vest

Refactors secbot code, cleans up bot code in general. Centralizes defines #8492

Refactors secbot code, cleans up bot code in general. Centralizes defines #8492

Conversation

Tsar-Salat commented Feb 13, 2023 • edited Loading

About The Pull Request

Why It's Good For The Game

Testing Photographs and Procedure

Changelog

Rukofamicom commented Feb 13, 2023

Tsar-Salat commented Feb 13, 2023 • edited Loading

Rukofamicom commented Feb 13, 2023

Tsar-Salat commented Feb 13, 2023 • edited Loading

Rukofamicom commented Feb 13, 2023 • edited Loading

Fronsis commented Feb 14, 2023

Tsar-Salat commented Feb 14, 2023 • edited Loading

PowerfulBacon left a comment

Choose a reason for hiding this comment

Tsar-Salat commented Feb 16, 2023

zeskorion commented Feb 20, 2023 • edited Loading

itsmeow commented Mar 7, 2023

github-actions bot commented Mar 11, 2023

Rukofamicom commented Oct 24, 2023 • edited Loading

Tsar-Salat commented Oct 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 26, 2023

Tsar-Salat commented Oct 26, 2023

Tsar-Salat commented Oct 26, 2023

Rukofamicom commented Oct 27, 2023

Tsar-Salat commented Oct 27, 2023

Rukofamicom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rukofamicom Oct 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Tsar-Salat commented Feb 13, 2023 •

edited

Loading

Rukofamicom commented Feb 13, 2023 •

edited

Loading

Tsar-Salat commented Feb 14, 2023 •

edited

Loading

zeskorion commented Feb 20, 2023 •

edited

Loading

Rukofamicom commented Oct 24, 2023 •

edited

Loading

Rukofamicom Oct 27, 2023 •

edited

Loading