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 custom hit-shapes & TargetableOffsets to TD structures #13460

Merged
merged 5 commits into from Jun 14, 2017

Conversation

Projects
None yet
5 participants
@reaperrr
Contributor

reaperrr commented Jun 5, 2017

I think the title and commit titles/descs are self-explanatory.

@reaperrr reaperrr added this to the Playtest featuring updated HitShapes milestone Jun 5, 2017

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 6, 2017

Member

Needs a rebase.

Member

pchote commented Jun 6, 2017

Needs a rebase.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 6, 2017

Member

Something we'll need to deal with, not sure if here or a followup, is that airstrikes can now one-shot a construction yard.

Member

pchote commented Jun 6, 2017

Something we'll need to deal with, not sure if here or a followup, is that airstrikes can now one-shot a construction yard.

@@ -309,6 +336,18 @@ HAND:
AFLD:
Inherits: ^BaseBuilding
HitShape:

This comment has been minimized.

@pchote

pchote Jun 6, 2017

Member

It's more work, I know, but i'm thinking for cases like this and the comm center it would make more sense and help balancing if we defined the hitbox based on the ground footprint, and then accounted for these bits sticking up by increasing the Z offset of the targetable position.

@pchote

pchote Jun 6, 2017

Member

It's more work, I know, but i'm thinking for cases like this and the comm center it would make more sense and help balancing if we defined the hitbox based on the ground footprint, and then accounted for these bits sticking up by increasing the Z offset of the targetable position.

This comment has been minimized.

@reaperrr

reaperrr Jun 6, 2017

Contributor

With 'ground footprint', do you mean occupied cells or visual footprint of the art?

@reaperrr

reaperrr Jun 6, 2017

Contributor

With 'ground footprint', do you mean occupied cells or visual footprint of the art?

This comment has been minimized.

@pchote

pchote Jun 6, 2017

Member

One or the other, but i'm not sure which is best. Artwork feels better but is more (possibly too much) work to set up and balance than occupied cells.

@pchote

pchote Jun 6, 2017

Member

One or the other, but i'm not sure which is best. Artwork feels better but is more (possibly too much) work to set up and balance than occupied cells.

This comment has been minimized.

@reaperrr

reaperrr Jun 6, 2017

Contributor

I just realized it might be worth it to account for higher-than-0 Z-offsets in the combat overlay, by drawing a line to the ground WPos, like we do for blocking height.

Anyway, I'll see what I can do about the airfield, but I'm not sure what changes you have in mind for the comm center (HQ or EYE, btw?). Target offsets are already footprint-based, and making the right half higher than the occupied cell is partially to cover the main buildings' sprite (not the adv. comm center antenna), partially to make the center target offset not sit at the edge/inner corner of the combined shape, otherwise attacks from north-east would have trouble dealing full damage if their spread is low and/or inaccuracy is high.

@reaperrr

reaperrr Jun 6, 2017

Contributor

I just realized it might be worth it to account for higher-than-0 Z-offsets in the combat overlay, by drawing a line to the ground WPos, like we do for blocking height.

Anyway, I'll see what I can do about the airfield, but I'm not sure what changes you have in mind for the comm center (HQ or EYE, btw?). Target offsets are already footprint-based, and making the right half higher than the occupied cell is partially to cover the main buildings' sprite (not the adv. comm center antenna), partially to make the center target offset not sit at the edge/inner corner of the combined shape, otherwise attacks from north-east would have trouble dealing full damage if their spread is low and/or inaccuracy is high.

@reaperrr reaperrr removed the PR: Rebase me! label Jun 6, 2017

@pchote

Looks good aside from those two questions, which i'm aware may be opening a can of worms.

Pinging @AoAGeneral so that you can start thinking about how to counteract the balance changes this brings.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 6, 2017

Contributor

Rebased.

Something we'll need to deal with, not sure if here or a followup, is that airstrikes can now one-shot a construction yard.

For once, it might actually be better to make some preliminary balance adjustments here (same for the RA PR), otherwise it might get difficult to get everything merged before the 3-week 'deadline' you mentioned (I won't have time this weekend, btw).

Contributor

reaperrr commented Jun 6, 2017

Rebased.

Something we'll need to deal with, not sure if here or a followup, is that airstrikes can now one-shot a construction yard.

For once, it might actually be better to make some preliminary balance adjustments here (same for the RA PR), otherwise it might get difficult to get everything merged before the 3-week 'deadline' you mentioned (I won't have time this weekend, btw).

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 6, 2017

Member

it might get difficult to get everything merged before the 3-week 'deadline' you mentioned (I won't have time this weekend, btw).

I don't see any chance of actually meeting that deadline, so lets do our best but not break ourselves over it.

Member

pchote commented Jun 6, 2017

it might get difficult to get everything merged before the 3-week 'deadline' you mentioned (I won't have time this weekend, btw).

I don't see any chance of actually meeting that deadline, so lets do our best but not break ourselves over it.

@AoAGeneral

This comment has been minimized.

Show comment
Hide comment
@AoAGeneral

AoAGeneral Jun 7, 2017

Contributor

I can start looking into it in the future. But a few questions:

Does AoE damage types greatly effect this? (IE: Grenades, nukes, flame weapons, all increase damage greatly because they hit multiple parts of the structure?)

Contributor

AoAGeneral commented Jun 7, 2017

I can start looking into it in the future. But a few questions:

Does AoE damage types greatly effect this? (IE: Grenades, nukes, flame weapons, all increase damage greatly because they hit multiple parts of the structure?)

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 7, 2017

Member

No, there is still only one part of the structure to hit. It's just bigger. The problem with the airstrikes is that they have multiple bombs spread out over an area - previously only the one closest to the middle of the structure did damage, but now all the ones that hit the artwork do.

Member

pchote commented Jun 7, 2017

No, there is still only one part of the structure to hit. It's just bigger. The problem with the airstrikes is that they have multiple bombs spread out over an area - previously only the one closest to the middle of the structure did damage, but now all the ones that hit the artwork do.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 7, 2017

Contributor

@AoAGeneral The only weapons that should now be more powerful vs. buildings that are larger than a single cell are:

  • airstrike weapons, due to the special nature of the attack (shooting in a line rather than at a single specific point)
  • weapons with max inaccuracy greater than 426 (ArtilleryShell, Grenade, 227mm)

All other weapons should be unaffected (well, maybe BoatMissile from certain directions, but that doesn't matter for MP balance).

Contributor

reaperrr commented Jun 7, 2017

@AoAGeneral The only weapons that should now be more powerful vs. buildings that are larger than a single cell are:

  • airstrike weapons, due to the special nature of the attack (shooting in a line rather than at a single specific point)
  • weapons with max inaccuracy greater than 426 (ArtilleryShell, Grenade, 227mm)

All other weapons should be unaffected (well, maybe BoatMissile from certain directions, but that doesn't matter for MP balance).

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 7, 2017

Member

Also

  • Weapons with a reasonable sized spread that are force-fired on the ground in the cell next to structures.

I consider that a good thing though, because it provides a bonus for creative tactics.

Member

pchote commented Jun 7, 2017

Also

  • Weapons with a reasonable sized spread that are force-fired on the ground in the cell next to structures.

I consider that a good thing though, because it provides a bonus for creative tactics.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 7, 2017

Contributor

Updated airfield and both comm centres.

Contributor

reaperrr commented Jun 7, 2017

Updated airfield and both comm centres.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Jun 7, 2017

Member

Power Plants also look like need adjustments. That top-right one may go a bit left.

power plant

Also comm centre has a target position out of its hitshape, is that intented? I tested with some units and damage output looks same but i'm not sure.

Member

MustaphaTR commented Jun 7, 2017

Power Plants also look like need adjustments. That top-right one may go a bit left.

power plant

Also comm centre has a target position out of its hitshape, is that intented? I tested with some units and damage output looks same but i'm not sure.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 7, 2017

Contributor

Also comm centre has a target position out of its hitshape, is that intented? I tested with some units and damage output looks same but i'm not sure.

Yes, it's intended. It looks like it's outside the shape because I moved it up the Z-axis, which the debug overlay doesn't show. If you want to know the 2D ground position, you just need to set the 3rd value of the 3rd offset on HQ from 256 to 0.
I'll file a separate PR for improved debug overlay which will show the Z height as well, but not in the next few days.

I'll fix the power plant offsets.

Contributor

reaperrr commented Jun 7, 2017

Also comm centre has a target position out of its hitshape, is that intented? I tested with some units and damage output looks same but i'm not sure.

Yes, it's intended. It looks like it's outside the shape because I moved it up the Z-axis, which the debug overlay doesn't show. If you want to know the 2D ground position, you just need to set the 3rd value of the 3rd offset on HQ from 256 to 0.
I'll file a separate PR for improved debug overlay which will show the Z height as well, but not in the next few days.

I'll fix the power plant offsets.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 7, 2017

Contributor

Updated:
pplants1

Contributor

reaperrr commented Jun 7, 2017

Updated:
pplants1

@pchote pchote added the PR: Needs +2 label Jun 11, 2017

@pchote

pchote approved these changes Jun 11, 2017

@rob-v

This comment has been minimized.

Show comment
Hide comment
@rob-v

rob-v Jun 12, 2017

Contributor

While testing another PR, I had this situation, I assume it is solved by this PR.

No damage by turrets:
obrazok

Edit - Seems to work with this PR:
obrazok

Contributor

rob-v commented Jun 12, 2017

While testing another PR, I had this situation, I assume it is solved by this PR.

No damage by turrets:
obrazok

Edit - Seems to work with this PR:
obrazok

Fix TD civ field husk footprint
Players should not be able to build on it.
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 12, 2017

Contributor

Rebased.

While testing another PR, I had this situation, I assume it is solved by this PR.

Yes. If you enable Debug menu in lobby options and then in the in-game menu toggle on Debug -> Show Combat Geometry, you can see the targetable offsets (green crosses) now being inside the hitboxes (yellow rectangles) of buildings (it only looks like it's outside for comm center/adv. comm center, because it's shifted up on the Z axis, but the 2d pos is actually inside).

Contributor

reaperrr commented Jun 12, 2017

Rebased.

While testing another PR, I had this situation, I assume it is solved by this PR.

Yes. If you enable Debug menu in lobby options and then in the in-game menu toggle on Debug -> Show Combat Geometry, you can see the targetable offsets (green crosses) now being inside the hitboxes (yellow rectangles) of buildings (it only looks like it's outside for comm center/adv. comm center, because it's shifted up on the Z axis, but the 2d pos is actually inside).

@AoAGeneral

This comment has been minimized.

Show comment
Hide comment
@AoAGeneral

AoAGeneral Jun 12, 2017

Contributor

Is it possible to have engineers and Commandos run to the target locations rather then the center?

Contributor

AoAGeneral commented Jun 12, 2017

Is it possible to have engineers and Commandos run to the target locations rather then the center?

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Jun 12, 2017

Member

That is implemented in #13495.

Member

pchote commented Jun 12, 2017

That is implemented in #13495.

reaperrr added some commits May 10, 2017

Increased ZOffset of TD explosions
To make sure they always play on top of actors.
@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Jun 12, 2017

Contributor

Updated, fixed the last two things @rob-v pointed out.

Contributor

reaperrr commented Jun 12, 2017

Updated, fixed the last two things @rob-v pointed out.

@rob-v

rob-v approved these changes Jun 12, 2017

taking into account also @MustaphaTR's review, for me also 👍

@reaperrr reaperrr merged commit 086fc88 into OpenRA:bleed Jun 14, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@reaperrr reaperrr deleted the reaperrr:TD-hitshapes1 branch Jul 23, 2017

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