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 of the legless cyborgs when leave transports #567

Open
wants to merge 51 commits into
base: develop
Choose a base branch
from

Conversation

FS-21
Copy link
Contributor

@FS-21 FS-21 commented Apr 2, 2022

And a new tag for enabling the old behavior, ideal for repair vehicles.

In the vanilla YR game legless Cyborgs recover legs when leave transports.

Summary by CodeRabbit

  • New Features
    • Introduced the ability for transports to repair legless cyborgs.
  • Bug Fixes
    • Fixed a bug that incorrectly restored cyborg legs when soldiers exited transports.
  • Documentation
    • Updated documentation to reflect new functionalities and bug fixes related to cyborg leg repairs.

and a new tag for enabling the old behavior, ideal for repair vehicles.
@github-actions
Copy link

github-actions bot commented Apr 2, 2022

Nightly build for this pull request:

@AlliedG
Copy link

AlliedG commented Apr 9, 2022

Omitted CanRepairCyborgLegs= tag from transport and cyborg legs were not reattached (1st image)

Added CanRepairCyborgLegs=true tag to transport and cyborg legs were reattached (2nd image)

https://imgur.com/a/MU3fW82

Interestingly damaged spark anims of cyborg constantly play, even if inside a transport.

@FS-21
Copy link
Contributor Author

FS-21 commented Apr 10, 2022

Looks related to OpenTopped logic because standard transports doesn't show sparks.

BUT from what I discovered in vanilla YR tests that "bug" is part of vanilla YR, not caused by this feature so we can consider that this " just works".

@AlliedG
Copy link

AlliedG commented May 6, 2022

This no longer requires testing as the sparking in transport due to opentopped is expected? I see some errors with merging?

@DmitryVolkov666
Copy link

Works.
But as for me it's better to expand the funcional
For example

1 [General] globally enable or disable the repair of cyborgs inside the transport (default - repair works)

2 [InfantryType] (cyborg=yes) add a pair of keys that prohibit or allow the cyborg to be repaired inside any vehicle (default - repairable)

3 [VehicleType] add a key to a unit that can repair a cyborg/cyborgs (you can try adding an enumeration). in this case, this priority will be higher than priority 2.

@Metadorius Metadorius force-pushed the develop branch 2 times, most recently from 0fa5476 to da80463 Compare May 11, 2022 17:01
@Metadorius
Copy link
Member

@FS-21 A friendly reminder to fix the issues.

@chaserli chaserli added the Needs Update This branch needs to be updated with upstream for testing and reviews label Sep 8, 2022
@chaserli chaserli removed the Needs Update This branch needs to be updated with upstream for testing and reviews label Oct 17, 2022
@FS-21
Copy link
Contributor Author

FS-21 commented Jul 12, 2024

Regarding the code rewrite I made after the last review I explained in the hook "InfantryClass_PerCellProcess_CyborgLegsCheck" that the InfantryClass values of "SequenceAnim" and "Crawling" values will be reset before entering into transports (I don't know if is related to the limbo process or whathever) so I still needed to save the legless value of the cyborg in the variable pTechnoExt->IsLeggedCyborg. Also here is placed the transport check for overwriting this behaviour if needed.

Lastly, in the unlimbo process (hook "FootClass_Unlimbo_LaserTrails") this value is checked for restoring the legs state.

@FS-21
Copy link
Contributor Author

FS-21 commented Aug 29, 2024

One question, I missed something here or all feedback was addressed?

Copy link
Member

@Metadorius Metadorius left a comment

Choose a reason for hiding this comment

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

Looks good to me. I am not sure on the tag name, previously I thought it only applies to Transporter. Maybe there should be some other tag name if it doesn't apply only to transporter? Like FixCyborgLegsWhenEntered.

Also is the rest of the hooks except the Limbo hook even necessary? I thought infantry goes limboed either way. Or did I miss something?

@Metadorius Metadorius added Fix and merge this and removed Needs Update This branch needs to be updated with upstream for testing and reviews Needs improvement labels Sep 26, 2024
@FS-21
Copy link
Contributor Author

FS-21 commented Sep 26, 2024

Looks good to me. I am not sure on the tag name, previously I thought it only applies to Transporter. Maybe there should be some other tag name if it doesn't apply only to transporter? Like FixCyborgLegsWhenEntered.

Also is the rest of the hooks except the Limbo hook even necessary? I thought infantry goes limboed either way. Or did I miss something?

that capability was expanded to structures so the title the PR wasn't updated
image

@Metadorius
Copy link
Member

@FS-21 I adjsuted the tag name. Please verify if removing any other hooks other than the first one (Limbo one) has any effect on your feature/fix. I suspect the limbo hook is enough to fully accomplish what you want.

@FS-21
Copy link
Contributor Author

FS-21 commented Oct 26, 2024

@FS-21 I adjsuted the tag name. Please verify if removing any other hooks other than the first one (Limbo one) has any effect on your feature/fix. I suspect the limbo hook is enough to fully accomplish what you want.

I checked and these 3 cases with cyborg infantry:

  • Entering in vehicle.
  • Entering in Structure (a bunker)
  • Entering in a Linked tunnel (Ares tunnels logic)
    And all have in common they execute the hook:
    DEFINE_HOOK(0x51DF42, InfantryClass_Limbo_Cyborg, 0x7)

The problem I see is you have no idea if the cyborg entered into a structure, only transport vehicles ("Transporter") or there are more tags in the limboed infantry related to structures that can be checked in this hook? if not then "DEFINE_HOOK(0x52291A, InfantryClass_InfantryEnteredThing_Cyborg, 0x6)" is for garrison into structures and "DEFINE_HOOK(0x51A27F, InfantryClass_PerCellProcess_AresTunnel_Cyborg, 0xA)" for the Tunnels logic from Ares.

@Metadorius
Copy link
Member

@FS-21 I adjsuted the tag name. Please verify if removing any other hooks other than the first one (Limbo one) has any effect on your feature/fix. I suspect the limbo hook is enough to fully accomplish what you want.

I checked and these 3 cases with cyborg infantry:

  • Entering in vehicle.
  • Entering in Structure (a bunker)
  • Entering in a Linked tunnel (Ares tunnels logic)
    And all have in common they execute the hook:
    DEFINE_HOOK(0x51DF42, InfantryClass_Limbo_Cyborg, 0x7)

The problem I see is you have no idea if the cyborg entered into a structure, only transport vehicles ("Transporter") or there are more tags in the limboed infantry related to structures that can be checked in this hook? if not then "DEFINE_HOOK(0x52291A, InfantryClass_InfantryEnteredThing_Cyborg, 0x6)" is for garrison into structures and "DEFINE_HOOK(0x51A27F, InfantryClass_PerCellProcess_AresTunnel_Cyborg, 0xA)" for the Tunnels logic from Ares.

I am not sure I understand what is he issue. All of those are TechnoClasses, so it should be irrelevant whether it enters a tunnel, a transport or whatever. You always have a TechnoTypeClass.

@FS-21
Copy link
Contributor Author

FS-21 commented Oct 28, 2024

@FS-21 I adjsuted the tag name. Please verify if removing any other hooks other than the first one (Limbo one) has any effect on your feature/fix. I suspect the limbo hook is enough to fully accomplish what you want.

I checked and these 3 cases with cyborg infantry:

  • Entering in vehicle.
  • Entering in Structure (a bunker)
  • Entering in a Linked tunnel (Ares tunnels logic)
    And all have in common they execute the hook:
    DEFINE_HOOK(0x51DF42, InfantryClass_Limbo_Cyborg, 0x7)

The problem I see is you have no idea if the cyborg entered into a structure, only transport vehicles ("Transporter") or there are more tags in the limboed infantry related to structures that can be checked in this hook? if not then "DEFINE_HOOK(0x52291A, InfantryClass_InfantryEnteredThing_Cyborg, 0x6)" is for garrison into structures and "DEFINE_HOOK(0x51A27F, InfantryClass_PerCellProcess_AresTunnel_Cyborg, 0xA)" for the Tunnels logic from Ares.

I am not sure I understand what is he issue. All of those are TechnoClasses, so it should be irrelevant whether it enters a tunnel, a transport or whatever. You always have a TechnoTypeClass.

I think you missed the point here: Transports & structures decide if legless cyborgs should be repairable if they enter, not vice-versa.

@Metadorius
Copy link
Member

I think you missed the point here: Transports & structures decide if legless cyborgs should be repairable if they enter, not vice-versa.

Yeah but your hook in Limbo already has the Transporter pointer. Won't that be enough to decide whether to repair legs or not?

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

Successfully merging this pull request may close these issues.

7 participants