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

Custom item support #93

Merged
merged 10 commits into from
Oct 19, 2024
Merged

Custom item support #93

merged 10 commits into from
Oct 19, 2024

Conversation

UltiNaruto
Copy link

Required mains supported for Missiles/PB
Unlimited Missiles/PBs are supported now
Spring Ball has its own item now

@UltiNaruto
Copy link
Author

Non required mains seeds need to set unknown1 (which is Unknown Item 2) in starting items to 12 (4 | 8 => Missile Launcher | Power Bomb Launcher).

If you only require mains for missile then it is 8.
If you only require mains for PB then it is 4.

@UltiNaruto
Copy link
Author

Migrated Ice Trap from Special Function to Pickup and it doesn't require enable_ice_trap in config anymore

src/patch_config.rs Show resolved Hide resolved
src/pickup_meta.rs Outdated Show resolved Hide resolved
src/pickup_meta.rs Outdated Show resolved Hide resolved
src/starting_items.rs Outdated Show resolved Hide resolved
src/patches.rs Outdated Show resolved Hide resolved
src/pickup_meta.rs Show resolved Hide resolved
src/pickup_meta.rs Outdated Show resolved Hide resolved
src/patches.rs Outdated Show resolved Hide resolved
src/pickup_meta.rs Show resolved Hide resolved
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

Need to update the JSON schema with all of your changes

@toasterparty
Copy link

@UltiNaruto PLEASE do not force-push when updating your changes. It makes it hard for me to incrementally review your work (otherwise I have to double over all lines of your code each feedback iteration)

@toasterparty
Copy link

Actually, I forgot you may have told me a way to see the old code for force push. It might be fine

@toasterparty
Copy link

@Miepee Miepee linked an issue Aug 14, 2024 that may be closed by this pull request
@UltiNaruto UltiNaruto force-pushed the main branch 2 times, most recently from cd0af93 to 68e78d8 Compare August 14, 2024 15:24
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

So things are less confusing moving forwards, can you rename unknown0 and unknown1 in spawn_point.rs to unknown1 and unknown2? We should be consistent with where we start count from and I think it's pretty well established we start at 1 when talking about inventory items,

Make sure the JSON input also reflects this change.

@UltiNaruto
Copy link
Author

So things are less confusing moving forwards, can you rename unknown0 and unknown1 in spawn_point.rs to unknown1 and unknown2? We should be consistent with where we start count from and I think it's pretty well established we start at 1 when talking about inventory items,

Make sure the JSON input also reflects this change.

unknown1 or unknownItem1?

@toasterparty
Copy link

unknownItem1

@UltiNaruto UltiNaruto force-pushed the main branch 2 times, most recently from bb7915a to 07f3241 Compare August 15, 2024 03:37
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

We're getting close. Thanks for sticking with me

schema/randomprime.schema.json Outdated Show resolved Hide resolved
schema/randomprime.schema.json Outdated Show resolved Hide resolved
schema/randomprime.schema.json Outdated Show resolved Hide resolved
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

I just noticed game_config.spring_ball_item isn't in the JSON schema. Can you please add that and then make it so that it defaults to the spring ball item if unspecified and game_config.spring_ball == false?

Also, placing a spring ball pickup while game_config.spring_ball == false doesn't do anything which is a little confusing. I think we should have the patcher panic if the user tries to place spring ball while spring ball is disabled

schema/randomprime.schema.json Outdated Show resolved Hide resolved
@toasterparty
Copy link

Actually, @UltiNaruto we should deprecated game_config.spring_ball and default game_config.spring_ball_item to the custom pickup.

Sorry for the flip flop, caffine still kicking in

@duncathan
Copy link

Actually, @UltiNaruto we should deprecated game_config.spring_ball and default game_config.spring_ball_item to the custom pickup.

Sorry for the flip flop, caffine still kicking in

Would it perhaps be preferable to default it to bombs, for backwards compatibility?

@toasterparty
Copy link

To preserve backwards compatibility, make it so that springBall: true sets springBallItem: Bombs.

@UltiNaruto
Copy link
Author

I thought about another idea. What about making springBall default to false and if it's set to true with a pickup having SpringBall as its type then it complains about both being used?

springBall would remain useable and not deprecated but using both makes the patcher panic with the following message Use either springBall or add a pickup as SpringBall type

As for using SpringBall type for pickups, it would detect it then change springBall to true and springBallItem to SpringBall.

@toasterparty
Copy link

@UltiNaruto I really think this is the best option, please implement it:

  1. Deprecate springBall in the JSON schema and remove all if config.spring_ball conditions
  2. Add springBallItem to the JSON schema with "default": "Spring Ball"
  3. Add the following
let spring_ball_item = match spring_ball_item {
    Some(spring_ball_item) => spring_ball_item,
    None => {
        match spring_ball {
            false => "Spring Ball",
            true => "Morph Ball Bombs",
        }
    },
}

@UltiNaruto
Copy link
Author

Up to you. I'll do that then.

@UltiNaruto UltiNaruto force-pushed the main branch 2 times, most recently from e14d103 to 9d424b8 Compare August 20, 2024 03:31
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

I'm pretty happy with the code but in my testing startingItems isn't working. Please check using this JSON file which should start with all the custom items.

{
    "$schema": "./schema/randomprime.schema.json",
    "gameConfig": {
        "startingRoom": "Tallon Overworld:Arbor Chamber",
        "startingItems": {
            "combatVisor": true,
            "powerBeam": true,
            "scanVisor": true,
            "missiles": 10,
            "energyTanks": 0,
            "powerBombs": 8,
            "wave": false,
            "ice": false,
            "plasma": false,
            "charge": false,
            "morphBall": true,
            "bombs": false,
            "spiderBall": false,
            "boostBall": false,
            "powerSuit": 0,
            "variaSuit": false,
            "gravitySuit": false,
            "phazonSuit": false,
            "thermalVisor": false,
            "xray": false,
            "spaceJump": true,
            "grapple": false,
            "superMissile": false,
            "wavebuster": false,
            "iceSpreader": false,
            "flamethrower": false,
            "unknownItem1": 0,
            "unlimitedMissiles": true,
            "unlimitedPowerBombs": true,
            "missileLauncher": true,
            "powerBombLauncher": true,
            "springBall": true
        }
    },
    "levelData": {
        "Tallon Overworld": {
            "rooms": {
                "Arbor Chamber": {
                    "pickups": [
                        {
                            "type": "Spring Ball"
                        },
                        {
                            "type": "Main Power Bomb",
                            "position": [-723.6977, 334.1136, 40.2428]
                        },
                        {
                            "type": "Missile Launcher",
                            "position": [-723.6977, 329.1136, 40.2428]
                        },
                        {
                            "type": "Unlimited Missiles",
                            "position": [-723.6977, 344.1136, 40.2428]
                        },
                        {
                            "type": "Unlimited Power Bombs",
                            "position": [-723.6977, 349.1136, 40.2428]
                        }
                    ]
                }
            }
        }
    }
}

Also, please update use the struct to define starting items rather than u64 so that the player spawns with missile/pb launcher if left unspecified

let starting_items = {
    let items = self.game_config.starting_items.as_ref();

    match items {
        Some(items) => items.clone(),
        None => {
            if force_vanilla_layout {
                StartingItems::from_u64(2188378143)
            } else {
                StartingItems::from_u64(1)
            }
        }
    }
};

schema/randomprime.schema.json Outdated Show resolved Hide resolved
schema/randomprime.schema.json Outdated Show resolved Hide resolved
schema/randomprime.schema.json Show resolved Hide resolved
@UltiNaruto UltiNaruto force-pushed the main branch 2 times, most recently from 57419f8 to b517b58 Compare August 30, 2024 13:29
Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

I found a bug with the InventoryActivator special function. It appears that any custom pickup will meet the activation requirement, not just the item id named in the special function object.

Here's an example where the path through Waterfall Cavern is supposed to be blocked until the player has "Missile Launcher" in their inventory, yet it allows the player through with only "Main Power Bomb":

{
    "$schema": "./schema/randomprime.schema.json",
    "gameConfig": {
        "startingRoom": "Tallon Overworld:Waterfall Cavern",
        "startingItems": {
            "combatVisor": true,
            "powerBeam": true,
            "scanVisor": true,
            "missiles": 10,
            "energyTanks": 0,
            "powerBombs": 3,
            "wave": false,
            "ice": false,
            "plasma": false,
            "charge": false,
            "morphBall": true,
            "bombs": false,
            "spiderBall": false,
            "boostBall": false,
            "powerSuit": 0,
            "variaSuit": false,
            "gravitySuit": false,
            "phazonSuit": false,
            "thermalVisor": false,
            "xray": false,
            "spaceJump": true,
            "grapple": false,
            "superMissile": false,
            "wavebuster": false,
            "iceSpreader": false,
            "flamethrower": false,
            "unknownItem1": 0,
            "unlimitedMissiles": false,
            "unlimitedPowerBombs": false,
            "missileLauncher": false,
            "powerBombLauncher": false,
            "springBall": false
        }
    },
    "levelData": {
        "Tallon Overworld": {
            "rooms": {
                "Waterfall Cavern": {
                    "pickups": [
                        {
                            "type": "Main Power Bomb",
                            "position": [-257.8459, 434.5406, -36.1156]
                        }
                    ],
                    "blocks": [
                        {
                            "id": 9000000,
                            "position": [-269.5781, 445.1988, -43.7749],
                            "scale": [10.0, 2.0, 5.0]
                        }
                    ],
                    "triggers": [
                        {
                            "id": 9000001,
                            "position": [-269.5781, 445.1988, -43.7749],
                            "scale": [15.0, 15.0, 30.0]
                        }
                    ],
                    "specialFunctions": [
                        {
                            "id": 9000002,
                            "type": "InventoryActivator",
                            "itemId": "Missile Launcher"
                        }
                    ],
                    "addConnections": [
                        {
                            "senderId": 9000001,
                            "state": "INSIDE",
                            "targetId": 9000002,
                            "message": "ACTION"
                        },
                        {
                            "senderId": 9000002,
                            "state": "ZERO",
                            "targetId": 9000000,
                            "message": "DEACTIVATE"
                        }
                    ]
                }
            }
        }
    }
}

If the fix for this isn't trivial, that's fine, we'll just remove the custom pickups from the special function itemId enum and move on.

@UltiNaruto
Copy link
Author

Fixed it. It was just missing a value & 1 to prevent the value to be higher than 1.

Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

Please make the Missile Launcher default to +5 missile capacity instead of +1, and Main Power Bomb +4 instead of +1

src/patches.rs Outdated Show resolved Hide resolved
src/patches.rs Outdated
Comment on lines 11348 to 11386
blt remove_floaty_jump;
li r5, 0x4000; // 4 << 12 => Fluid Count = 0b100
or r4, r4, r5;
lis r5, 0x41a0; // 20.0
b apply_floaty_jump;
remove_floaty_jump:
li r5, 0x7000; // 7 << 12 => Fluid Count = 0b111
nor r5, r5, r5; // flip bits
and r4, r4, r5; // set Fluid Count to 0b000
lis r5, 0;
apply_floaty_jump:
stw r4, { actor_flags_offset }(r3);
stw r5, { fluid_depth_offset }(r3);
b end_init_power_up;

Choose a reason for hiding this comment

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

This needs to account for if the player is already underwater. The way you have implemented it has several problems:

  • if you receive floaty jump while underwater, the distance underwater is ruined making it harder to jump out of water without gravity suit
  • if you receive floaty jump while underwater, your fluid count is set to 1 instead of 2, meaning you can't re-activate floaty by leaving a water box through it's side
  • if you remove floaty jump while underwater, it incorrectly gives the player "psuedo-grav" properties

Note that there is also some places in vanilla where it is acceptable to have fluid count of 2 (crashed frigate).

Copy link
Author

@UltiNaruto UltiNaruto Sep 17, 2024

Choose a reason for hiding this comment

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

if adding floaty jump :

  • if out of water ticks not 2 then jump to end_init_power_up
  • else set fluid count to 1 and fluid depth to 20.0

else :

  • if out of water ticks not 2 then jump to end_init_power_up
  • else set fluid count to 0

would it be good like that?

Choose a reason for hiding this comment

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

What is out of water ticks? The number of frames the player is not submerged?

Copy link
Author

Choose a reason for hiding this comment

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

Most likely. It's checked a lot when parsing surface restraint and always checks against 2 which is fully out of water.

Copy link
Author

Choose a reason for hiding this comment

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

That's the check that is often used that I mentioned above : https://github.com/AxioDL/metaforce/blob/main/Runtime/World/CPlayer.hpp#L599-L601

Copy link
Author

Choose a reason for hiding this comment

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

this still needs confirmation that this is what you want

Choose a reason for hiding this comment

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

Here's my test JSON. The underwater depth count seems correct, but we're still not getting the correct fluid count. If you collect the "Remove Floaty Jump" under the water, you should not get "psuedo-grav". If you collect the floaty jump under the water and then leave the water, you should still have a non-zero fluid count.

{
    "$schema": "./schema/randomprime.schema.json",
    "gameConfig": {
        "startingRoom": "Tallon Overworld:Landing Site",
        "startingItems": {
            "combatVisor": true,
            "powerBeam": true,
            "scanVisor": true,
            "missiles": 10,
            "energyTanks": 0,
            "powerBombs": 3,
            "wave": false,
            "ice": false,
            "plasma": false,
            "charge": false,
            "morphBall": true,
            "bombs": false,
            "spiderBall": false,
            "boostBall": false,
            "powerSuit": 0,
            "variaSuit": false,
            "gravitySuit": false,
            "phazonSuit": false,
            "thermalVisor": false,
            "xray": false,
            "spaceJump": true,
            "grapple": false,
            "superMissile": false,
            "wavebuster": false,
            "iceSpreader": false,
            "flamethrower": false,
            "unknownItem1": 0,
            "unlimitedMissiles": false,
            "unlimitedPowerBombs": false,
            "missileLauncher": false,
            "powerBombLauncher": false,
            "springBall": false
        }
    },
    "levelData": {
        "Tallon Overworld": {
            "rooms": {
                "Landing Site": {
                    "pickups": [
                        {
                            "type": "Floaty Jump",
                            "position": [-369.6311, 374.9892, -20.7154]
                        },
                        {
                            "type": "Floaty Jump",
                            "scanText": "Remove Floaty Jump",
                            "maxIncrease": -1,
                            "position": [-369.6311, 360.9892, -20.7154]
                        },
                        {
                            "type": "Floaty Jump",
                            "position": [-334.3455, 319.7977, -39.3822]
                        },
                        {
                            "type": "Floaty Jump",
                            "scanText": "Remove Floaty Jump",
                            "maxIncrease": -1,
                            "position": [-334.3455, 311.7977, -39.3822]
                        }
                    ]
                }
            }
        }
    }
}

Copy link
Author

@UltiNaruto UltiNaruto Sep 25, 2024

Choose a reason for hiding this comment

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

should work with this code now :

// check if it is floaty jump
        check_floaty_jump:
            cmpwi        r29, { PickupType::FloatyJump.kind() };
            bne          continue_init_power_up;
            lwz          r3, 0x84c(r30);
            lwz          r0, { out_of_water_ticks_offset }(r3);
            lwz          r5, { actor_flags_offset }(r3);
            mr           r4, r5;
            srwi         r5, r5, 14; // remove bits on the right of fluid count
            andi         r5, r5, 7;  // remove bits on the left of fluid count

            // remove fluid counts
            lis          r6, 0xffff;
            ori          r6, r6, 0x3fff;
            and          r4, r4, r6;

            cmpwi        r14, 0;
            blt          remove_floaty_jump;
            addi         r5, r5, 1;  // add 1 to fluid count
            andi         r5, r5, 7;  // making sure we don't go past 3 bits (=> 0b111)
            slwi         r5, r5, 14;
            or           r4, r4, r5; // actor flags |= (value << 14)
            cmpwi        r0, 2;      // check if we are in water
            bne          apply_underwater_floaty_jump;
            lis          r5, 0x41a0; // 20.0
            b            apply_floaty_jump;
        remove_floaty_jump:
            cmpwi        r5, 0;
            ble          do_not_decrement_fluid_count;
            addi         r5, r5, -1; // subtract 1 to fluid count
        do_not_decrement_fluid_count:
            andi         r5, r5, 7;  // making sure we don't go past 3 bits (=> 0b111)
            slwi         r5, r5, 14;
            or           r4, r4, r5; // actor flags |= (value << 14)
            cmpwi        r0, 2;      // check if we are in water
            bne          apply_underwater_floaty_jump;
            lis          r5, 0;
        apply_floaty_jump:
            stw          r5, { fluid_depth_offset }(r3);
        apply_underwater_floaty_jump:
            stw          r4, { actor_flags_offset }(r3);
            b            end_init_power_up;

Copy link
Author

Choose a reason for hiding this comment

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

this still needs confirmation that this is what you want

Choose a reason for hiding this comment

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

I thought I made it very clear what conditions need to be met and even went to the lengths of providing you with test JSON that can be used to validate those conditions:

If you collect the "Remove Floaty Jump" under the water, you should not get "psuedo-grav". If you collect the floaty jump under the water and then leave the water, you should still have a non-zero fluid count.

It's not my responsibility to test your changes. Push your changes if you think they are ready and I will review, otherwise test it yourself if you're not sure.

Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

  • Fix FJ implementation or back the changes out
  • Implement default item id for special functions without using a string literal

Copy link

@toasterparty toasterparty left a comment

Choose a reason for hiding this comment

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

image

Collect this item as the first and only item. After acquisition, the player must not have full influence underwater (i.e. they must not be able to move as if they have gravity suit without actually having gravity suit)

@UltiNaruto
Copy link
Author

UltiNaruto commented Oct 13, 2024

It works as intended for underwater remove floaty but it does +2 for fluid count when picking up non removal one. I'll fix that one.

@UltiNaruto
Copy link
Author

Well nvm it wasn't recompiled to latest version. I'll add a out of water ticks check to fluid count decrement.

@toasterparty toasterparty merged commit 8ecd4bd into randovania:main Oct 19, 2024
3 checks passed
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.

Feature: Missile/Power Bomb launchers / Required Mains
4 participants