Some refactoring needed?

[NiLSPACE]

it works

[Lapayo]

- Too many small files. Makes compilation take (quite a lot) longer.
- Using "char" instead of "NIBBLETYPE" for metas. currently, metas are handled as unsigned chars.
- Not using "override" for overridden virtual functions. Please, get used to this one, it's a really good practice to mark overridden functions, the compiler then checks if the function is really overriding something, and thus can catch some nasty typo bugs.

I´ll try to stick to the last 2 proposals.

But about those many small files:
I have no idea how to speed them up. Putting them all back in one file wouldn´t be a solution, because it would mess everything up again.

[xoft]

I think a simple grouping will be a benefit. For example, group together growables, tools, redstone-related, travel etc.

[Lapayo]

Is the cutback in compilation time really so extreme and fatal that we have to take this limitation?

[xoft]

I don't know, I just feel that files that have a total of 10 lines of code aren't worth it, it's better to have "dense" files. One-file-per-class-no-matter-at-what-cost feels too java-ish to me.
Also, I'd expect that most changes are done not to a single block, but to a naturally related group of blocks, which makes the naturally grouped files reasonable.

[xoft]

I'm afraid there's one more problem with the refactoring - now the project won't compile on Mac OS X, and possibly other *nixes. GCC complains that it cannot pass a temporary cItem object to a function expecting a cItem reference:

Code:

In file included from source/items/Item.cpp:10:
source/items/ItemSlab.h: In member function ‘virtual bool cItemSlabHandler::OnItemUse(cWorld*, cPlayer*, cItem*, int, int, int, char)’:
source/items/ItemSlab.h:26: error: no matching function for call to ‘cInventory::RemoveItem(cItem)’
source/items/../cInventory.h:35: note: candidates are: bool cInventory::RemoveItem(cItem&)
In file included from source/items/Item.cpp:15:
source/items/ItemBucket.h: In member function ‘virtual bool cItemBucketHandler::OnItemUse(cWorld*, cPlayer*, cItem*, int, int, int, char)’:
source/items/ItemBucket.h:37: error: no matching function for call to ‘cInventory::RemoveItem(cItem)’
source/items/../cInventory.h:35: note: candidates are: bool cInventory::RemoveItem(cItem&)
source/items/ItemBucket.h:40: error: no matching function for call to ‘cInventory::AddItem(cItem)’
source/items/../cInventory.h:34: note: candidates are: bool cInventory::AddItem(cItem&)
source/items/ItemBucket.h:55: error: no matching function for call to ‘cInventory::RemoveItem(cItem)’
source/items/../cInventory.h:35: note: candidates are: bool cInventory::RemoveItem(cItem&)
source/items/ItemBucket.h:64: error: no matching function for call to ‘cInventory::AddItem(cItem)’
source/items/../cInventory.h:34: note: candidates are: bool cInventory::AddItem(cItem&)
In file included from source/items/Item.cpp:20:
source/items/ItemDye.h: In member function ‘virtual bool cItemDyeHandler::OnItemUse(cWorld*, cPlayer*, cItem*, int, int, int, char)’:
source/items/ItemDye.h:25: error: no matching function for call to ‘cInventory::RemoveItem(cItem)’
source/items/../cInventory.h:35: note: candidates are: bool cInventory::RemoveItem(cItem&)
source/items/Item.cpp: In member function ‘virtual void cItemHandler::PlaceBlock(cWorld*, cPlayer*, cItem*, int, int, int, char)’:
source/items/Item.cpp:240: error: no matching function for call to ‘cInventory::RemoveItem(cItem)’
source/items/../cInventory.h:35: note: candidates are: bool cInventory::RemoveItem(cItem&)
make: *** [build/debug/source/items/Item.o] Error 1

[Lapayo]

I thought this is an allowed way of passing an object O.o
I changed it and the compiler shouldn´t complain anymore..