JimmyCushnie / JECS

Jimmy's Epic Config System
Do What The F*ck You Want To Public License
154 stars 16 forks source link

Allow class members to set their serialized name with the DoSave attribute #27

Closed pipe01 closed 4 years ago

JimmyCushnie commented 4 years ago

Instead of making DoSave dual-purpose, should we instead have a separate SaveAs attribute?

pipe01 commented 4 years ago

I think I prefer how it is, having two attributes per member may be slightly cumbersome

JimmyCushnie commented 4 years ago

Remember you can combine attributes, like so: [DoSave, SaveAs("exampleName")]. I think it's good practice to have separate attributes for separate purposes

pipe01 commented 4 years ago

How about if you have the SaveAs attribute you don't need the DoSave one

JimmyCushnie commented 4 years ago

Hmm. I'm not a huge fan, since that's again combining two purposes into one attribute.

However if the attributes are separate, it seems unintuitive for something marked with SaveAs to not be saved.

OK, here's my proposal. Keep the system you've made here, with DoSave being used both to save private members and to set the name of a saved member. However, rename the attribute's Name property to SaveAs, and remove the attribute constructor so that you're forced to explicitly declare what the string does.

[DoSave(SaveAs: "ShortName")]
private int StupidNameWhichIsVeryLongAndStupid;

I'm also thinking DoSave and DontSave should be renamed to SaveThis and DontSaveThis.

pipe01 commented 4 years ago

Sounds good

JimmyCushnie commented 4 years ago

Awesome stuff, if you can just move ClassMember to the SUCC.ParsingLogic namespace I'll merge :)

pipe01 commented 4 years ago

I did! The commit isn't shown here for whatever reason

JimmyCushnie commented 4 years ago

Ah so you did! My bad lol.

Thanks for a great PR!