Skip to content

fix: dead/powered down barricade is not predicted#3136

Merged
cu-kai merged 1 commit intoUnvanquished:masterfrom
cu-kai:cu-kai/bcpredict
Oct 22, 2024
Merged

fix: dead/powered down barricade is not predicted#3136
cu-kai merged 1 commit intoUnvanquished:masterfrom
cu-kai:cu-kai/bcpredict

Conversation

@cu-kai
Copy link
Copy Markdown
Contributor

@cu-kai cu-kai commented Oct 20, 2024

take 2

{
BG_BuildableBoundingBox( ent->modelindex, bmins, bmaxs );
if ( ent->modelindex == BA_A_BARRICADE && ent->torsoAnim == BANIM_IDLE_UNPOWERED )
if ( ent->modelindex == BA_A_BARRICADE && ( ent->torsoAnim != BANIM_IDLE1 || !( ent->eFlags & EF_B_SPAWNED ) ) )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking closer at my original fix, I wonder if we could just do ent->legsAnim == BANIM_POWERDOWN as the condition and catch everything...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems to work ok in my tests?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

eh, it's meh. I think add a note here saying // TODO: This is not perfect and can be improved.

@slipher
Copy link
Copy Markdown
Contributor

slipher commented Oct 21, 2024

BarricadeComponent::HandleDie references four animations: BANIM_DESTROY, BANIM_DESTROYED, BANIM_DESTROY_UNPOWERED, and BANIM_DESTROYED_UNPOWERED. None of which has been mentioned so far!

In retrospect, maybe it was a bad idea to make cgame compute buildable bounds from their config files. IIRC I did this because of incorrect prediction with some buildables that had non-integral heights. The prediction was wrong because it used bounding box info sent from the surface which was rounded to integers. But we could just change the bounds to be integers instead. Maybe sending the stuff from the server is a good tradeoff in favor of simplicity (against not allowing non-integer bounds and sending more data on the network).

@DolceTriade
Copy link
Copy Markdown
Member

Ok, before doing some extensive redesign, I think it's fair to say this improves the situation. Perhaps it is fine to submit as is to improve the situation and then we can implement the full design after?

@slipher
Copy link
Copy Markdown
Contributor

slipher commented Oct 22, 2024

I'm not demanding the use of any specific approach. But if anyone is interested in trying the old version with bboxes calculated by the server, they can revert 7f00ec8.

@DolceTriade
Copy link
Copy Markdown
Member

LGTM

@cu-kai cu-kai merged commit 853523a into Unvanquished:master Oct 22, 2024
@cu-kai cu-kai deleted the cu-kai/bcpredict branch October 22, 2024 18:57
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.

3 participants