Remove some dead code (maybe a merge defect?)

[bencsikandrei]

• small formatting changes
• small rearrange

[bencsikandrei]

There's a couple of other changes that could be made in these files.

But I need some guidance, please, whether you'd like me to try them out or not.

One example is the class layout. As it is now, if I counted correctly, on a 64 bit arch the animated class takes 72B (it's virtual so, 8 bytes bonus)
int starting_frame_time_;
bool does_not_change_; // Optimization for 1-frame permanent animations
bool started_;
bool force_next_update_;
// 1 B padding
std::vector frames_;
int max_animation_time_;
int start_tick_;
bool cycles_;
// 7B paddding
double acceleration_;
int last_update_tick_;
int current_frame_key_;

Some small rearrangements, like so:
int starting_frame_time_;
bool does_not_change_; // Optimization for 1-frame permanent animations
bool started_;
bool force_next_update_;
bool cycles_;
std::vector frames_;
int max_animation_time_;
int start_tick_;
double acceleration_;
int last_update_tick_;
int current_frame_key_;

Takes it down to 64, which fits a cache line.

Other things can be envisioned, like double -> float, but the alignment requirements will make it so that that doesn't help. Maybe some of the ints (4B) can be smaller ints? Maybe I'm going too far :D

Is this something of interest?

Have a good one!

[bencsikandrei]

In the same spirit as another PR I made, maybe a separate PR with replacing typedef with using would be interesting?

[bencsikandrei]

It is pretty late here, but I'm not missing something here, right?

animated is set to 1, at line 80, then never changed and then checked for <= 0

I guess it was a simple merge request gone weird

[stevecotton]

Your change looks good to me. Looks as if this should have been removed in 20bf7fa, when this function stopped taking the acceleration as a parameter.

[Vultraz]

Can you put these two conditions on one line.

[bencsikandrei]

done

[Pentarctagon]

Takes it down to 64, which fits a cache line.

Other things can be envisioned, like double -> float, but the alignment requirements will make it so that that doesn't help. Maybe > some of the ints (4B) can be smaller ints? Maybe I'm going too far :D

Is this something of interest?

While optimizations are good, I'd say this is overkill unless the performance improvements are measurable in some way.

[CelticMinstrel]

Did we have a policy regarding braces on if statements? I forget. It's not that I'm opposed to adding them there, mind you… just mixing style changes with not-style changes is something I prefer not to happen. Although… these may not be style changes but they are basically non-functional changes, other than making the constructor explicit.

Takes it down to 64, which fits a cache line.
Other things can be envisioned, like double -> float, but the alignment requirements will make it so that that doesn't help. Maybe > some of the ints (4B) can be smaller ints? Maybe I'm going too far :D
Is this something of interest?

While optimizations are good, I'd say this is overkill unless the performance improvements are measurable in some way.

Or to put it another way… profile it.

[bencsikandrei]

@Pentarctagon I'll try to see the impact it has. If the struct is used very often or some vector if it iterated often it's probably worth it

@CelticMinstrel I didn't mean to change that one if, I thought I undid that change before the commit and push, whoops! I'm on board with consistency also. Hell of a reflex with the {}... working on it

@Pentarctagon @CelticMinstrel if you have some quick tips on how I can benchmark effectively and be quick about, I'd highly appreciate it, otherwise I'm gonna continue on my road to knowing the project and will figure it out eventually :D Thanks!

I'll make the changes you suggested ASAP, thanks for the feedback!

[CelticMinstrel]

Just FTR, I don't mind adding the braces there… if I'm hesitant of anything, it's adding them in the same commit (or PR?) as this other stuff. It's only one change, so it's not that big a deal, but still.

[Pentarctagon]

@bencsikandrei There's profiler support in cmake and scons if you're on Linux, otherwise I don't have any advice on it though.