hello again, welcome back!

Now it should compile... This journey will cause a lot of static_casts to be added, as int indexes seem to be the standard...

Could you change BrewingstandEntity::GetResultItem, SignEntity::Get/SetLine to size_t?

Of course I can. Just didn't do it, to not to break/change too many files. I would also change GetSlot to unsigned, as this mainly prevented me from simply converting the whole loop.

Sure do it

Hey @bibo38 you still working on this?

I've stopped after reworking the whole ItemGrid class to work consistently with size_t for slots and then realized that this required too much code changes in every other part, which would create a massive PR (again). Then I've put it aside.
Now I'm checking, how much code changes are necessary to change the GetItem signature alone (maybe I can even get away with overloading 😏 ).

@tigerw Or did you started with some major work in your continuation PR?

Nothing major, I say add what you have here, I don't mind a large list of changes :)
After we can recombine the two PRs

I don't mind a large list of changes :)

I just want to prevent the scenario that happened to #4226. Better getting a bunch of small changes into master which are quick to check.

How big are the changes when you use size_t for item grids? If you have something ready just push.

By the way, is it worth using std array absolutely everywhere? Compared to a native language feature (C arrays) the templates will have more of an impact on compile times, no?

They were really big with the whole ItemGrid class, I threw those changes away. With only GetItem(int) it's around ~60 lines affected. But I'm currently on the track of just overriding the GetItem call with size_t, as this just adds one static_cast (otherwise I need to add it to those 60 lines, because reworking those code parts changes other function signatures or requires a bunch of casts to int).

By the way, is it worth using std array absolutely everywhere? Compared to a native language feature (C arrays) the templates will have more of an impact on compile times, no?

Looking at the (compilation) speed is a preliminary optimization. Unless someone benchmarks a significant speed impact, it isn't worth to optimize, as these optimizations take their price:

• Inconsistency
  • You need to agree and document when to use which format
  • You have to review the correct usage (if everything is std::array then Clang-Tidy does this for us)
  • Code changes may require switching the format to be consistent with the documented standard
  • Similar code changes in multiple files may not be that easy to copy/paste, as std::array has other features than C-Arrays (like getting the size)
  • There are slight differences, like the requirement for unsigned index in STL-Containers
• std::array just fits a lot better into the C++ programming style
  • a = b copies the array (already used in this PR)
    • You could replace/substitute the whole CopyXXX methods with the autogenerated operator= of the compiler.
  • Comparison operators can work with the underlying types, so std::array<cItem, 9> allows you to easily compare against other arrays.
  • size() to get the count, one fewer macro
  • for(auto x : container) to loop over the elements

Also C++ limits the compilation speed, as it needs to be backwards compatible to C89 and C++98 (otherwise they could've just replaced C-Arrays), so most of the new stuff is added as a template, as these constructs may also be used to port functionality back to older standards. Changing the whole project to C would probably speed up the compilation more than all C++ optimizations combined. Or one could just code everything in Python, Javascript or Lua and only write the performance bottlenecks in C or similar, which gives nearly zero compilation time 😏 .

P.S.: I also wouldn't really expect that much of a slowdown, as the header for std::array was already included (i didn't add any headers 🙈 ). I've had some experience in another project, where in a small codebase reducing the includes (of a header-only library) to the necessary includes saved about 10s.

Just pushed my current state... Some Lua stuff seems to go crazy. I've to investigate if this is caused by the overloading (probably) or by the changes to the bindings.

You might need a cLuaState::Push overload for size_t.

also thanks for the rundown on std::array, agreed any impact is probably negligible.