[ER] QOL binding for retrieving local player's PlayerIns#272
[ER] QOL binding for retrieving local player's PlayerIns#272ArloFilley wants to merge 6 commits intovswarte:mainfrom
Conversation
crates/eldenring/src/cs/chr_ins.rs
Outdated
| /// the caller must ensure that no other references to [`WorldChrMan`] are | ||
| /// held at the time of calling. This method mutably borrows [`WorldChrMan`] | ||
| /// internally to reach `main_player`. | ||
| unsafe fn instance() -> InstanceResult<&'static mut Self> { |
There was a problem hiding this comment.
Maybe we should implement this as a new method PlayerIns::main_player() as well as for DS3? I don't really feel like it's intuitive to have PlayerIns::instance() give you the main player thinking about it now. Thoughts?
There was a problem hiding this comment.
It feel intuitive to me that PlayerIns::instance returns the main player. I do agree however that it should be immediatly obvious to everyone what PlayerIns::instance is returning, which is not currently the case. Personally I'd like to keep the current pattern, but I'm not sure there's a way to make the return type more explicit without a wrapper around PlayerIns. Unless I'm missing something?
If there isn't a way to make the return type more obvious then a new main_player() method is the way to go.
There was a problem hiding this comment.
main_player() (or local_player()?) makes sense to me too. issue isn't the return type, it's that there are multiple player instances for NPCs and networked players but instance() implies it's a singleton
There was a problem hiding this comment.
the main player also doesn't have a static lifetime. it's reallocated when reloading the game, so maybe this shouldn't be a FromStatic anyways
There was a problem hiding this comment.
I kind of agree that this is stretching the definition a bit. What do you think @nex3?
There was a problem hiding this comment.
fwiw I think PlayerIns::local_player() would be more appropriate than PlayerIns::main_player()
There was a problem hiding this comment.
the main player also doesn't have a static lifetime. it's reallocated when reloading the game, so maybe this shouldn't be a
FromStaticanyways
This is also the case for several much broader types that are currently FromStatic, like MapItemMan. We definitely want a global way to access these singleton instances, but they don't follow the requirement that the 'static lifetime last the remaining duration of the program. This is probably worth a separate discussion.
For this case, I'm fine with changing the name here and in DS3 to local_player().
Adds two small quality-of-life methods to the eldenring crate, mirroring functionality in the DS3 crate.
+
PlayerIns::InstanceAdds a manual
FromStaticimpl forPlayerInsthat returns the local main player viaWorldChrMan.main_player. This creates a simple binding for accessing the main player. Useful for finding players stats, data, and position.Mirrors previously existing implementation
fromsoftware-rs/crates/darksouls3/src/sprj/chr_ins.rs
Lines 250 to 264 in 25b909c
+
CS::ChrIns.kill()Adds a kill() method to ChrIns and all subclasses. Kills characters by setting current hp (
ChrIns.module_container.data.hp) to 0.Mirrors previously existing implementation
fromsoftware-rs/crates/darksouls3/src/sprj/chr_ins.rs
Lines 177 to 188 in 25b909c