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.

Sign up for GitHub

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

Parse misc item info #53

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Parse misc item info #53

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73e3bf6
Parse misc item info
NickCondron Feb 21, 2023
67480bb
Rename ChargeLaunched to ChargeState
NickCondron Mar 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions peppi/src/model/enums/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,5 +232,26 @@ pseudo_enum!(Type: u16 {
236 => BIRDO_EGG,
});

pseudo_enum!(TurnipType: u8 {
0 => SMILEY,
1 => BORED,
2 => SLEEPY,
3 => SHOCKED,
4 => LAUGHING,
5 => WINK,
6 => DOT_EYES,
7 => STITCH_FACE,
});

pseudo_enum!(MissileType: u8 {
0 => HOMING,
1 => SUPER,
});

pseudo_enum!(ChargeLaunched: u8 {
NickCondron marked this conversation as resolved.
Show resolved Hide resolved
0 => CHARGING,
1 => LAUNCHED,
});

pseudo_enum!(State: u8 {
});
12 changes: 10 additions & 2 deletions peppi/src/model/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
use serde::Serialize;

use crate::model::{
enums::item::{State, Type},
enums::item::{ChargeLaunched, MissileType, State, TurnipType, Type},
primitives::{Direction, Port, Position, Velocity},
};
use peppi_derive::Arrow;

#[derive(Clone, Copy, Debug, PartialEq, Serialize, Arrow)]
pub struct MiscInfo {
pub missile_type: MissileType,
pub turnip_type: TurnipType,
pub charge_launched: ChargeLaunched,
pub charge_power: u8,
}
Copy link
Owner

Choose a reason for hiding this comment

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

What advantage do you see in encapsulating these inside a MiscInfo struct, as opposed to splatting them into Item?

Copy link
Contributor Author

@NickCondron NickCondron Feb 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Because the single Option<MiscInfo> in Item encodes the fact that you either have all four fields or none at all because they are all from the same Slippi version. They seem somewhat logically grouped to me too because they all describe specific properties of subsets of items. I wouldn't die on this hill though.

Copy link

Choose a reason for hiding this comment

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

From an user perspective, they're pretty distinct imo. They won't always know that they came from the same slippi version. The 2 charge values being lumped makes sense, but it's an odd place to look for missiles and turnips which are unrelated to charge shots and eachother.


#[derive(Clone, Copy, Debug, PartialEq, Serialize, Arrow)]
pub struct Item {
pub id: u32,
Expand All @@ -26,7 +34,7 @@ pub struct Item {

#[serde(skip_serializing_if = "Option::is_none")]
#[slippi(version = "3.2")]
pub misc: Option<[u8; 4]>,
pub misc_info: Option<MiscInfo>,

#[serde(skip_serializing_if = "Option::is_none")]
#[slippi(version = "3.5")]
Expand Down
75 changes: 46 additions & 29 deletions peppi/src/serde/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
},
frame::{self, Post, Pre},
game::{self, Netplay, Player, PlayerType, MAX_PLAYERS, NUM_PORTS},
item::Item,
item::{Item, MiscInfo},
primitives::{Port, Position, Velocity},
slippi, triggers,
},
Expand Down Expand Up @@ -512,39 +512,56 @@ fn frame_end(r: &mut &[u8]) -> Result<FrameEvent<FrameId, frame::End>> {
}

fn item(r: &mut &[u8]) -> Result<FrameEvent<FrameId, Item>> {
let id = FrameId::new(r.read_i32::<BE>()?);
trace!("Item Update: {:?}", id);
let frame_id = FrameId::new(r.read_i32::<BE>()?);
trace!("Item Update: {:?}", frame_id);
let r#type = item::Type(r.read_u16::<BE>()?);
let state = item::State(r.read_u8()?);
let direction = {
let x = r.read_f32::<BE>()?;
if x == 0.0 {
None
} else {
Some(x.try_into()?)
}
};
let velocity = Velocity {
x: r.read_f32::<BE>()?,
y: r.read_f32::<BE>()?,
};
let position = Position {
x: r.read_f32::<BE>()?,
y: r.read_f32::<BE>()?,
};
let damage = r.read_u16::<BE>()?;
let timer = r.read_f32::<BE>()?;
let id = r.read_u32::<BE>()?;
// v3.2
let misc_info = if_more(r, |r| {
Ok(MiscInfo {
missile_type: item::MissileType(r.read_u8()?),
turnip_type: item::TurnipType(r.read_u8()?),
charge_launched: item::ChargeLaunched(r.read_u8()?),
charge_power: r.read_u8()?,
Copy link
Owner

Choose a reason for hiding this comment

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

How sure are you that these fields are always/only used for these purposes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They aren't, but other potential uses are unknown or undocumented. I'm pretty sure the data these fields are written from is part of a big union that has different values for different items as shown in the decomp. That's why they are garbage when the item doesn't match.

https://github.com/doldecomp/melee/blob/master/src/melee/it/types.h#L655

https://github.com/project-slippi/slippi-ssbm-asm/blob/master/Recording/SendItemInfo.s#L118

In theory, the exact byte that describes the peach turnip type could be the exact byte that determines some property of some other item, but that would be hard to find out.

Copy link
Owner

Choose a reason for hiding this comment

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

They aren't, but other potential uses are unknown or undocumented. I'm pretty sure the data these fields are written from is part of a big union that has different values for different items as shown in the decomp. That's why they are garbage when the item doesn't match.

If the meanings of these fields vary based on context, I don't think it makes sense to give them semantic names even if there's a "typical" use.

In theory we could provide some way to interpret values contextually instead, but that seems overkill for four independent(?) bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand not giving them their "typical" name, but otherwise the best name would be something like item_0xd4 referring to the byte offset and then provide uses in documentation.

})
})?;
// v3.6
let owner = if_more(r, |r| Ok(Port::try_from(r.read_u8()?).ok()))?;

Ok(FrameEvent {
id: id,
id: frame_id,
event: Item {
r#type: r#type,
state: item::State(r.read_u8()?),
direction: {
let x = r.read_f32::<BE>()?;
if x == 0.0 {
None
} else {
Some(x.try_into()?)
}
},
velocity: Velocity {
x: r.read_f32::<BE>()?,
y: r.read_f32::<BE>()?,
},
position: Position {
x: r.read_f32::<BE>()?,
y: r.read_f32::<BE>()?,
},
damage: r.read_u16::<BE>()?,
timer: r.read_f32::<BE>()?,
id: r.read_u32::<BE>()?,
r#type,
state,
direction,
velocity,
position,
damage,
timer,
id,
// v3.2
misc: if_more(r, |r| {
Ok([r.read_u8()?, r.read_u8()?, r.read_u8()?, r.read_u8()?])
})?,
misc_info,
// v3.6
owner: if_more(r, |r| Ok(Port::try_from(r.read_u8()?).ok()))?,
owner,
},
})
}
Expand Down
6 changes: 5 additions & 1 deletion peppi/src/serde/ser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,11 @@ fn item<W: Write>(w: &mut W, i: &item::Item, ver: Version, frame_idx: i32) -> Re
w.write_u32::<BE>(i.id)?;

if ver.gte(3, 2) {
w.write_all(&i.misc.unwrap())?;
let misc = &i.misc_info.unwrap();
w.write_u8(misc.missile_type.0)?;
w.write_u8(misc.turnip_type.0)?;
w.write_u8(misc.charge_launched.0)?;
w.write_u8(misc.charge_power)?;
}

if ver.gte(3, 6) {
Expand Down
23 changes: 19 additions & 4 deletions peppi/tests/peppi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use peppi::{
DashBack, End, EndMethod, Frames, Game, Language, Netplay, Player, PlayerType, Scene,
ShieldDrop, Start, Ucf,
},
item::Item,
item::{Item, MiscInfo},
metadata::{self, Metadata},
primitives::{Direction, Port, Position, Velocity},
slippi::{Slippi, Version},
Expand Down Expand Up @@ -573,7 +573,12 @@ fn items() {
timer: 140.0,
r#type: item::Type::PEACH_TURNIP,
velocity: Velocity { x: 0.0, y: 0.0 },
misc: Some([5, 5, 5, 5]),
misc_info: Some(MiscInfo {
missile_type: item::MissileType(5),
turnip_type: item::TurnipType::WINK,
charge_launched: item::ChargeLaunched(5),
charge_power: 5,
}),
owner: Some(Some(Port::P1)),
}
);
Expand All @@ -591,7 +596,12 @@ fn items() {
timer: 140.0,
r#type: item::Type::PEACH_TURNIP,
velocity: Velocity { x: 0.0, y: 0.0 },
misc: Some([5, 0, 5, 5]),
misc_info: Some(MiscInfo {
missile_type: item::MissileType(5),
turnip_type: item::TurnipType::SMILEY,
charge_launched: item::ChargeLaunched(5),
charge_power: 5,
}),
owner: Some(Some(Port::P1)),
}
);
Expand All @@ -609,7 +619,12 @@ fn items() {
timer: 140.0,
r#type: item::Type::PEACH_TURNIP,
velocity: Velocity { x: 0.0, y: 0.0 },
misc: Some([5, 0, 5, 5]),
misc_info: Some(MiscInfo {
missile_type: item::MissileType(5),
turnip_type: item::TurnipType::SMILEY,
charge_launched: item::ChargeLaunched(5),
charge_power: 5,
}),
owner: Some(Some(Port::P1)),
}
);
Expand Down