feat: Allow renaming of Plants and Gotchi #18
Conversation
| @@ -11,6 +12,9 @@ use spawn::spawn; | |||
| mod hatch; | |||
| use hatch::hatch; | |||
|
|
|||
| mod rename_gotchi; | |||
| use rename_gotchi::rename; | |||
There was a problem hiding this comment.
would complain because all of them are named i.e. "hatch" and not "hatch_item", but because gotchi aren't items I can see why you named it "rename_gotchi". okay.
There was a problem hiding this comment.
I changed the name of the Ask to NicknameGotchi, this should probably be nickname_gotchi but it really doesn't matter
|
|
||
| impl fmt::Display for Error { | ||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
| write!(f, "couldn't spawn items: ")?; |
There was a problem hiding this comment.
I thought we were renaming gotchi, not spawning items?
|
|
||
| #[derive(Debug)] | ||
| pub enum Error { | ||
| NoSuchItemConf(ConfigError), |
There was a problem hiding this comment.
I don't see how this error could occur outside of the context of spawning items ...
| let mut bobstead = Hackstead::register().await?; | ||
|
|
||
| // we'll need to keep track of how many items we have to see if spawning works. | ||
| fn count_relevant_items(hackstead: &Hackstead) -> usize { |
There was a problem hiding this comment.
not sure this actually tests renaming things, like it claims to
| @@ -10,6 +10,9 @@ use summon::summon; | |||
| mod rub; | |||
| use rub::rub; | |||
|
|
|||
| mod nickname; | |||
| use nickname::nickname; | |||
There was a problem hiding this comment.
the other one is named rename_gotchi, but this one is named "nickname". I know this is a bit of a nitpick, but could we be consistent here for maintenance's sake?
| let not_seed_item = not_seed_arch.spawn().await?; | ||
| let open_tile = bobstead.free_tile().expect("fresh hackstead no open land?"); | ||
|
|
||
| struct NewPlantAssumptions { |
There was a problem hiding this comment.
I'm going to go out on a limb here and say this test doesn't test renaming plants, it tests spawning them ...
There was a problem hiding this comment.
I would suggest probably going off of i.e. rub_effects tests here, since those just panic if planting a plant goes wrong, whereas these go across every case, sometimes expecting errors, etc.
|
There's an issue where the builds can fail or pass. It seems like an issue with bincode being too large to pass updates down the wormhole https://builds.sr.ht/~muirrum/job/289485#task-stable-1271 it also seems that the server is closing too early. |
| @@ -1,5 +1,6 @@ | |||
| use super::{strerr, HandledAskKind, SessSend}; | |||
| use crate::wormhole::server; | |||
| use hcor::id::ItemId; | |||
There was a problem hiding this comment.
I would merge this with the imports below it, but ok
| @@ -11,6 +12,9 @@ use spawn::spawn; | |||
| mod hatch; | |||
| use hatch::hatch; | |||
|
|
|||
| mod rename_gotchi; | |||
| use rename_gotchi::rename; | |||
There was a problem hiding this comment.
I changed the name of the Ask to NicknameGotchi, this should probably be nickname_gotchi but it really doesn't matter
| mod test { | ||
| #[actix_rt::test] | ||
| /// NOTE: requires that at least one item exists in the config! | ||
| async fn spawn() -> hcor::ClientResult<()> { |
There was a problem hiding this comment.
this probably shouldn't be named spawn
| } | ||
| } | ||
|
|
||
| pub fn rename(ss: &mut SessSend, item_id: ItemId, new_name: String) -> Result<String, Error> { |
There was a problem hiding this comment.
again, nickname would be better here just to match up with GotchiNickname (which I changed so it would be the same as PlantNickname)
| mod test { | ||
| #[actix_rt::test] | ||
| /// NOTE: relies on item/spawn! | ||
| async fn summon() -> hcor::ClientResult<()> { |
There was a problem hiding this comment.
doesn't seem to test nicknaming anywhere?
There was a problem hiding this comment.
and has a lot of summoning going on
| let seed_item = seed_arch.spawn().await?; | ||
| let open_tile = bobstead.free_tile().expect("New hackstead no open land?"); | ||
|
|
||
| let plant = open_tile.plant_seed(seed_item).await?; |
There was a problem hiding this comment.
hold on what are seeds doing in my gotchi test
#16