MCHPR / MCHPRS

A multithreaded Minecraft server built for redstone.
MIT License
1.56k stars 67 forks source link

Remove use of nightly features #86

Closed fee1-dead closed 1 year ago

fee1-dead commented 1 year ago
MalbaCato commented 1 year ago

https://github.com/MCHPR/MCHPRS/blob/97743175c8e7e354ee53487607fe0acdb581d03b/crates/core/src/blocks/mod.rs#L30-L59

Isn't this better as

 trait BlockTransform { 
     fn rotate(&mut self, amt: crate::blocks::RotateAmt) { 
         // ...
     } 
     fn rotate90(&mut self) {}; 
     fn flip(&mut self, dir: crate::blocks::FlipDirection) {}; 
 } 

 macro_rules! noop_block_transform { 
     ($($ty:ty),*$(,)?) => { 
         $( 
             impl BlockTransform for $ty; 
         )* 
     }; 
 }

? it would also allow easily implementing blocks that do rotate, but who's flip is a no-op, for example.

fee1-dead commented 1 year ago

This is just a design choice that the person who wrote this did not choose to have default no-op bodies. AFAIK you can't do impl Tr for Ty; and you can only do impl Tr for Ty {}.

MalbaCato commented 1 year ago

well, is it a good design choice? considering that's basically what the macro (previously blanket impl) is doing? and yes, probably should have code that compiles 😅

fee1-dead commented 1 year ago

Also the fact that the code compiles after this change shows that manual impls all have both flip and rotate90. So having default bodies is of dubious utility unless there is evidence that this is desired.

Olek47 commented 1 year ago

Can you quickly update the building part of README?

fee1-dead commented 1 year ago

Done. I haven't touched the CI configs and Docker stuff.

fee1-dead commented 1 year ago

@StackDoubleFlow is this something that is desired? Requiring nightly would make it harder to package/build on linux distributions. Also if a user uses distribution provided rust then they can only use stable.

RaitoBezarius commented 1 year ago

Friendly ping, we are trying to package MCHPRS in Nixpkgs and nightly features makes it harder to package it.