I want people's opinions on practice program I made

[endingphantom]

So in order to help me learn Rust I made a practice program. GitHub - ceaselessphantom/hammerwatch-builder-practice: In order to help learn Rust I worked on a builder/calculator for the game heroes of hammerwatch. It is not finished nor tested that much and I do not intend to finish it. This is just so people can tell me areas I could improve on for future projects.
I intend to move on to other stuff and leave it unfinished. It also has not been tested that much. I just want a general idea on if there are better ways I could have gone about handling data structures and similar things.

This program was meant to let the user change levels, class, items, and some other stuff but they were one of the last steps. User in the program refers to user of the end program. Since this is not finished they have not been implemented. Also I changed structure file last, item second last, and class way earlier so there is some disconnect there. I did not make it in a working state and instead tested individual pieces in playground since I did not want to handle error testing for a program I already kept rewriting.

For how I planned it to work my plan was for character to contain all the stats and then there to be a function for every way the stats could be modified. I would then use these functions in other functions that had pointers to them. These would be stored for things like items so that they could be activated or undone depending on items being removed or conditions being met.

[quinedot]

That repository is empty.

On the upside, I see nothing I would have done differently.

[endingphantom]

.. That is embarrassing. I remade it and forgot to upload the files again.

[scottmcm]

The first thing I notice is this:

ceaselessphantom/hammerwatch-builder-practice/blob/5c48de0394e12ecc1dcda5a8fd0865325f3e6592/char_strctures.rs#L1

You don't need to wrap files in mod foo { ... }, since files are already modules when you bring them into the project with mod foo; (for foo.rs).

See Separating Modules into Different Files - The Rust Programming Language for more.

[droundy]

It doesn't look like you've got this set up to be built using cargo. I'd say that would be first priority. Does it even compile?

	fn amulet_of_resistance(mut character: &mut Character) -> Item{
			fn this(mut character: &mut Character, yes: bool, layers: u16){
				change_flat(character.stats.resist, 5, yes, layers);
			}
			let these = Modify::NoIndex(this);
			let modifies = Modification{
				conditions: Questions::NoCondition,
				changes: these,
				activated: 0,
			};
			let one = add_modification(&mut character, modifies);
			let noun = "Amulet of Resistance".to_string();
			let tune = Attuneable::Possible(false);
			let rare = 0;
			return Item{name: noun, mod_indexs: vec![one], attune: tune, rarity: rare};
		}

You've got a lot of mut character: &mut Character which is almost always wrong. You aren't likely to want to change the character to be a reference to a different character, are you? I'd think that the compiler would warn you about an unneeded mut like this.

Change noun to name and tune to attune and your code will be more clear and can stutter less with Item { name, attune, ...

Looks like this and these are not used and should be cut. I'd expect the compiler to inform you of this also.

[quinedot]

There's not a full crate here (lib.rs or main.rs) so I'll just browse in lexical order. It's going to be a lot of little notes you could also gleam from reading the book rather than an overall project design review.

char_strctures.rs

• pub enum Questions holds a reference, so it will need to be generic over some lifetime
  • If Questions is not a short-lived data structure, you normally want to avoid this
  • If you don't need to lengthen or shorten the Vec, &mut [u16] is more idiomatic
• When I saw pub item_count: Parameter_Insert,//this the user can not manually edit., I wondered why it was pub then.
  • But maybe you're not referring to the user of the code
• Looking at fn initilize_character
  • You can't return a reference to something you created locally (unless you leak the memory forever, but don't do that). This should just return Character. It should perhaps be an associated function of Character.
  • Stat should clearly have an associated constructor function
    • fn new<S: Into<String>>(text: S, amount: f64) -> Self
  • Perhaps Stats should implement Default
  • Similar for PercentageStat and PercentageStats, etc
  • If all six implemented Default, you could just derive Default for Character
• On mutable parameters
  • You don't need mut x: &mut Type unless you're going to change what x points to within the function. You probably just wanted x: &mut Type.
  • Similarly elsewhere in the code base, you don't need to return mut Type.
• On iterating over Vecs and the like
  • I see a lot of for i in 0..thing.len() { use(vector[i]); }
  • Use iteration instead
    • for x in field.mod_indexs.iter().copied() { use(x); }
• In change_percentage in particular
  • Indexing based on the original length while removing elements is going to lead to bad times
  • Probably you want something with Vec::retain here

class.rs

• fn change_class
  • It looks like you wanted to compare the current name with the target name, but are trying to create new_name out of thin air?
  • You have a match statement here with no default case. Based on what it's doing, I'm going to guess that you have a set number of character classes (the ones listed). Instead of using Strings here, you should make an enum. Then you can have methods to get the name as a String from the enum, return their base stats, try to make the enum from a string of the name, etc.
• More generally, if everything is going to be hard-coded into the binary, consider using enums and &'static str in some fashion.
• But also consider having these things in some external resource you load at run time instead of hard-coded into the library. Instead of matching you'd probably use a HashMap, etc.

items.rs

• It looks like you're making a bunch of functions that operate similarly. I'm guessing this is why you were storing so many function pointers elsewhere.
• I see Questions in here again, but they have owned Vec<String>s instead of mutable references to Vec<u16>, so there's a bit of disconnect
• This is really crying out for some sort of higher-level abstraction...
• ...so maybe look into ECS (Entity Component Systems) more generally -- there are a few Rust-based videos and tutorials around

[endingphantom]

I meant to just return character. I messed that up abit quickly trying to solve basic errors before posting them here. In general though I was not asking how to solve errors as I said it is not finished. The only way I saw other than reference character a bunch was make it static. The programs in the function needed fields from it. I could reference specific parts but that would quickly make calling the functions cluttered and more difficult for pretty much no gain.
Okay I saw that specific scenario regarding classes in enums. That was meant to get input from the user that is why it is asking for a string. I did not look at the class file since it was so longed since I worked on it I largely forgot about it.
I'll add a description of what this program was meant to do.
One other note I only got to items but there were other things in this game that would use some of those functions that I was planning to write into the program. That is also the reason I did some things to try and make the program more flexible in how it could handle inputs.

[droundy]

@endingphantom

. In general though I was not asking how to solve errors as I said it is not finished.

This code was way more than you should be writing without making sure it compiles. The rust compiler gives very good feedback, provided you heed it rather than ploughing ahead regardless. When the compiler's concerns are hard to follow, this forum is great, but you'll get better advice with a program with better contained errors, i.e. with smaller code and sooner.

[endingphantom]

Removing the errors would not make the code work. I would still need to finish a good amount of the program. I wanted to actually work on a smaller project than this game but I failed to come up with a good one. Though I probably could have made it so items could be added in a really rough manner before posting it here when I decided I no longer wanted to work on the project.
For testing I had been testing individual bits in playground and it does not take me that long to track down errors. The structures bit on its own I had cleared of error messages before posting here aside missing things since I had not made or included them. The compiler did not give me much as they were syntax errors since I tested most things I wanted to do before or around adding it to the program.

[droundy]

This doesn't should like a good way to write a program. You don't want to be copying individual bits into a different file to see if they work. Much better you build up bit by bit code that works. There are stages where it won't compile as you write a single function or a single data structure, of course, but that's about all the time that a program should not be able to compile. (This advice applies to other languages also, although in dynamically typed languages there is somewhat less benefit to it.

[cole-miller]

A big advantage of writing programs the way droundy describes is that the compiler will help you catch architectural problems early, so you don't notice them for the first time only when you're already written a whole bunch of code that relies on some broken interface. The point is not to get to a fully "working" program sooner, rather to deal with problems spotted by the compiler early in the coding process.

It might be helpful to know that you can stub out the implementation of a function, or part of the implementation, like this:

fn do_it(x: &str) -> Result<Foo, Error> {
    todo!("do the thing")
}

[endingphantom]

I'll keep that some in mind. To my knowledge however there is no such issues in the code aside the disparity between the item file not being updated and this originated from talking about a syntax error.
I would prefer the discussion move on at this point.

[erelde]

@endingphantom

To my knowledge however there is no such issues in the code

Try to compile one of your files.

People are trying to help you. Using a standard project layout, using cargo to manage compilation will help you, fixing compile errors will help you. And it will help others help you.

[crumplecup]

The rust book coins a term I like a lot, compiler driven development, even if it is a bit derivative. The advice of those here echoes the line from the text:

Make the changes in Listing 20-12 to src/main.rs , and then let’s use the compiler errors from cargo check to drive our development.

[endingphantom]

.. I said I did before I even made this thread. At best I could just comment out the variables referring to things not yet present. It seems since this argument started it seems it is the only thing people are looking at.
Oh one other thing. I did not come here asking for help to make the code work as I said I got to burned out working on it. Normally I would have just finished the code but the program was bigger than I expected. This feels less an effort to help me and more like an argument has dominated the whole thread.

[steffahn]

I’d say it’s really unclear what kind of help you are asking for here. The Rust compiler is often described a being very helpful. Thus, if you don’t want any feedback on what to do to make your code compile on how to get your code to compile, you’re essentially opting out of the most helpful kind of feedback.

Also, in general, lots of tips or feedback on Rust code relates to what the compiler accepts and what it doesn’t, what patterns play nicely with the borrow checker and which don’t, etc… it’s kind-of a precondition to have either code that already compiles, or code that almost works, but you’re struggling with e.g. some borrow-checker errors, in order to get the most meaningful / useful feedback. If it’s all in a state where everything we’d notice when reading your code is things that the compiler already points out anyways, it’s hard to give useful feedback beyond either saying the same thing that rustc (invoked through cargo) or clippy could tell you as well, or telling you to just use these tools yourself (the latter option requiring a lot less time spent on our end).

As @cole-miller said above (pointing out e.g. todo), making your program compile is not the same as making it work. It’s understandable that you do not want to “make the code work”, but even then you could try to make most of it compile, favorably even with as few warnings or clippy lints firing as possible.

In any case, it’s also entierly understandable that you don’t want to go there (making it compile) with this particular code anymore either; the point where we’re trying to help you is in addressing the question of “how did you get there?”, the advise of @droundy is mostly speaking about how you could/should approach your next Rust project, i.e. try to regularly get back into a state where the code compiles. On the order of minutes… if it has been 10 minutes, or even 20 minutes working on changes, you should consider trying to make the code compile again. If it’s already been a hour, it’s due time to pause everything else and to start working on making everything compile again. There’s probably lots of people that wouldn’t go for 5 minutes without “asking the compiler for help” (which is essentially the same as starting to work on making the code compile again, since that’s the compiler’s main concern when it’s “helping” you).

If you’re referring to unimplemented stuff in your code, add a stub function for it (with todo), or a stub variable initialized with todo!() or with some dummy value, or add a stub struct for it (e.g. without any fields and a TODO comment) if it’s a type. If you have a few functions not passing the borrow checker, comment their function bodies out and add a temporary todo!() before or after each commented out function body, in order to make sure that the remainder of the program can still compile. (Rust is designed in a way such that the implementation of one function (usually) doesn’t make a difference for whether a different function compiles or not; so if you have a bunch of not-compiling-but-commented-out functions, you can eventually fix them one-by-one, keeping a program that compiles successfully after each step, and never needing to go back to a function that you already got working.) There’s probably more tricks that I can’t think of right now for making unfinished / broken code be able to still compile.

This approach has lots of benefits, to name a few

• the compiler can give useful feedback; but some errors can prevent other (helpful) error messages from being reached in the first place – and even if the one compiler message that could help you right now is still being emitted, it can easily drown in a sea of unrelated errors from other parts of the program.
• refactoring becomes easy. You’ll probably regularly choose one approach over another arbitrarily, or just try something out during programming. When running into a corner where you figure out certain things should’ve been done differently in the first place, refactoring becomes necessary. Once you’ve started refactoring and changed something, it’s insanely useful to let compiler error messages guide on what needs to change as-well…
• IDEs – if you use an IDE, for example something using rust-analyzer, you will only get help like auto-completion, displaying documentation, automatic refactoring, etc… if your code is compiling or in an “almost compiling successfully” state. It can also sometimes be useful to generate documentation locally, which also doesn’t work on too-broken code (generally, rustdoc ignores function bodies, but besides those, many other things must not be broken in order for you to successfully generate documentation). Running cargo doc --open locally can help you get an overview over your own modules, types, and functions, and it also includes local documentation of all your dependencies which has super useful implication like being able to search through all your dependencies at once with a single search bar.

[endingphantom]

I ended up here because the project was bigger than I intended to deal with, not that useful a program and only made for practice, and I was burned out on working on it. I never said I could not complete the code.
" I just want a general idea on if there are better ways I could have gone about handling data structures and similar things."
I meant ways to make the structures more organized than how I set them up. Not how I could set the code up in a way that just works. There are other factors to making a good program than just making a program that does not give errors.
I went through some of the errors and I guess I was to unfocused since I missed more than I thought. I cleared them out except for ones in Rust I do not know the syntax for and was planning on working out later. (I had already gone through a bunch and wanted to focus through other parts of practice at the time.)

[droundy]

@endingphantom

I meant ways to make the structures more organized than how I set them up.

I think the problem is that with a large code that is so incomplete we can't tell how you set them up or what you were intending them to do.

I made the mistake of looking through the largest file which didn't seem to have any code that would do anything, and the file clearly couldn't compile. It has just one function which does nothing. Clearly it was intended to do something, but it's hard to say what.

The items file did define some data structures, but they referred to other undefined types. Perhaps you need help with structuring your crate? I'd suggest starting by putting all your code in a single module in a single file. That will make it easier to refer to data structures in other data structures.