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

Makes sabotaged things respect friendly fire setting #363

Open
wants to merge 2 commits into
base: beta
from

Conversation

Projects
None yet
2 participants
@DexterHaslem
Member

DexterHaslem commented Oct 6, 2018

This makes sentry gun and dispenser damage given/received check friendly fire is on in respect to the saboteur's team. fixes #327

I cannot spell saboteur correctly

@DexterHaslem DexterHaslem requested a review from squeek502 Oct 6, 2018

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 6, 2018

Member

this might cover #273 as well, I will check if that explosion code checks the same gamerules code

Member

DexterHaslem commented Oct 6, 2018

this might cover #273 as well, I will check if that explosion code checks the same gamerules code

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 7, 2018

Member

oh great, this doesnt work for malicious sabotage of SG because no saboteur info is set in that case to distinguish them. I need to change that code to have a proper flag and still keep track of sab team first.. this is WIP

Member

DexterHaslem commented Oct 7, 2018

oh great, this doesnt work for malicious sabotage of SG because no saboteur info is set in that case to distinguish them. I need to change that code to have a proper flag and still keep track of sab team first.. this is WIP

@squeek502

This comment has been minimized.

Show comment
Hide comment
@squeek502

squeek502 Oct 7, 2018

Member

I'm not really seeing how #327 was happening in the first place. It looks like the previous code was essentially, "if an SG is sabotaged, anyone can damage it," and IsSabotaged() is a prereq of IsMaliciouslySabotaged(), which means that IsSabotaged() must include maliciously sabotaged SGs, right? So I'm confused. Did you confirm #327 was real?

Member

squeek502 commented Oct 7, 2018

I'm not really seeing how #327 was happening in the first place. It looks like the previous code was essentially, "if an SG is sabotaged, anyone can damage it," and IsSabotaged() is a prereq of IsMaliciouslySabotaged(), which means that IsSabotaged() must include maliciously sabotaged SGs, right? So I'm confused. Did you confirm #327 was real?

@@ -2087,27 +2087,45 @@ bool CFFGameRules::FCanTakeDamage( CBaseEntity *pVictim, CBaseEntity *pAttacker
// if it's a buildable, then we use the buildable's owner to perform the team checks etc. -> Defrag
CBasePlayer *pBuildableOwner = NULL;
bool isFriendlyFireOn = friendlyfire.GetInt() != 0;

This comment has been minimized.

@squeek502

squeek502 Oct 7, 2018

Member

GetBool() would make more sense here

@squeek502

squeek502 Oct 7, 2018

Member

GetBool() would make more sense here

This comment has been minimized.

@DexterHaslem
@DexterHaslem

DexterHaslem Oct 7, 2018

Member

doh!

}
if ( pVictim && pVictim->Classify() == CLASS_SENTRYGUN )
{
CFFSentryGun *pSentry = dynamic_cast <CFFSentryGun *> (pVictim);
// Allow team to kill their own SG if it is sabotaged
if (pSentry && pSentry->IsSabotaged())
return true;
if ( pSentry && pSentry->IsSabotaged() )

This comment has been minimized.

@squeek502

squeek502 Oct 7, 2018

Member

Haven't thought this through fully, but it seems like it should be possible to condense most/all of these if blocks into a single block and treat all buildables the same as appropriate, right? Check if Classify() == CLASS_SENTRYGUN || Classify() == CLASS_DISPENSER, then just cast to CFFBuildableObject or w/e, and all the code inside the if could be the same.

@squeek502

squeek502 Oct 7, 2018

Member

Haven't thought this through fully, but it seems like it should be possible to condense most/all of these if blocks into a single block and treat all buildables the same as appropriate, right? Check if Classify() == CLASS_SENTRYGUN || Classify() == CLASS_DISPENSER, then just cast to CFFBuildableObject or w/e, and all the code inside the if could be the same.

This comment has been minimized.

@DexterHaslem

DexterHaslem Oct 7, 2018

Member

yes, strongly agree. I would have made it shorter but i didnt spend the time. Ill try to

@DexterHaslem

DexterHaslem Oct 7, 2018

Member

yes, strongly agree. I would have made it shorter but i didnt spend the time. Ill try to

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 7, 2018

Member

It definitely exists (just sab a SG then shoot it), thats what i was trying to address. I assumed the sabotage properties would always be set on the buildable base class, which is not true, I think. I will re-test

Member

DexterHaslem commented Oct 7, 2018

It definitely exists (just sab a SG then shoot it), thats what i was trying to address. I assumed the sabotage properties would always be set on the buildable base class, which is not true, I think. I will re-test

@squeek502

This comment has been minimized.

Show comment
Hide comment
@squeek502

squeek502 Oct 7, 2018

Member

Where does FCanTakeDamage return false in that instance (when trying to damage your maliciously sabotaged SG)? Or is it something else that's affecting things?

Member

squeek502 commented Oct 7, 2018

Where does FCanTakeDamage return false in that instance (when trying to damage your maliciously sabotaged SG)? Or is it something else that's affecting things?

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 7, 2018

Member

The previous code always returned true if it was sabotaged, is that what you are asking?

Member

DexterHaslem commented Oct 7, 2018

The previous code always returned true if it was sabotaged, is that what you are asking?

@squeek502

This comment has been minimized.

Show comment
Hide comment
@squeek502

squeek502 Oct 7, 2018

Member

Yeah, but if that's true, shouldn't it be impossible for there to be a bug where a (maliciously) sabotaged SG couldn't be damaged? Maybe I'm misunderstanding what #327 is saying?

Member

squeek502 commented Oct 7, 2018

Yeah, but if that's true, shouldn't it be impossible for there to be a bug where a (maliciously) sabotaged SG couldn't be damaged? Maybe I'm misunderstanding what #327 is saying?

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 9, 2018

Member

Indeed, it's saying it could be damaged by the sabotuer which arguably doesn't make sense when friendly fire is on. this really comes down to a gameplay decision:

do sabotaged buildings 'belong' to the sabotaging team? I would not be surprised if the original devs intentionally made them 'building versus the world' when sabd

Member

DexterHaslem commented Oct 9, 2018

Indeed, it's saying it could be damaged by the sabotuer which arguably doesn't make sense when friendly fire is on. this really comes down to a gameplay decision:

do sabotaged buildings 'belong' to the sabotaging team? I would not be surprised if the original devs intentionally made them 'building versus the world' when sabd

@squeek502

This comment has been minimized.

Show comment
Hide comment
@squeek502

squeek502 Oct 11, 2018

Member

Ohhh, ok I get it. The spy (and their team) can damage the gun they have maliciously sabotaged when FF is off. The title of #327 is (edit: was) super confusing.

Member

squeek502 commented Oct 11, 2018

Ohhh, ok I get it. The spy (and their team) can damage the gun they have maliciously sabotaged when FF is off. The title of #327 is (edit: was) super confusing.

@DexterHaslem

This comment has been minimized.

Show comment
Hide comment
@DexterHaslem

DexterHaslem Oct 13, 2018

Member

True that! I'm traveling and will finish this up after next week

Member

DexterHaslem commented Oct 13, 2018

True that! I'm traveling and will finish this up after next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment