@Solarus:
Good point. Using an existing project can save a lot of time and effort since you gain years worth of code. However, the down side to that is you also gain years worth of code. ;) Has anyone looked at the code yet? Are binary formats what we'd need, or do they have to be replaced? Do tilesets or tilemaps work wonky? How does scripting work, both internally and in scripts? Is it fast, or slow, or what? What problems are there with the code? What is not a problem with the code? Has it even been assessed?
To be completely honest I looked at only a few files a few years ago then stopped there because it looked like "typical c++ garbage, where everything is a string, a map, or a map strings." I don't mean any disrespect to the authors, the project itself looks great and I might even play it once it is complete.
I just took another unbiased look and it looks like I am about to rip Solarus a new one :) , but keep in mind I do think it's a cool project and I am doing this not to be a dick, but because I am looking seriously at it as something the community might be a part of.
I looked first in something low-level to see how other code gets and uses data.
Code:
namespace {
const std::map<ResourceType, std::string> resource_type_names = {
{ ResourceType::MAP, "map" },
{ ResourceType::TILESET, "tileset" },
{ ResourceType::SPRITE, "sprite" },
{ ResourceType::MUSIC, "music" },
{ ResourceType::SOUND, "sound" },
{ ResourceType::ITEM, "item" },
{ ResourceType::ENEMY, "enemy" },
{ ResourceType::ENTITY, "entity" },
{ ResourceType::LANGUAGE, "language" },
{ ResourceType::FONT, "font" }
};
Really?! This is typical c++ at it's finest. Of course this NEEDS to be a map, and it NEEDS to be dynamically allocated strings that hold a "name" that is used to lookup other things. Because efficiency is not OOP enough.
Code:
QuestResources::QuestResources() {
for (size_t i = 0 ; i < resource_type_names.size(); ++i) {
resource_maps.emplace(static_cast<ResourceType>(i), ResourceMap());
}
}
Oh of course, it's a map of resource maps...
Code:
const std::string& QuestResources::get_resource_type_name(ResourceType resource_type) {
return resource_type_names.at(resource_type);
}
const std::map<ResourceType, std::string>& QuestResources::get_resource_type_names() {
return resource_type_names;
}
This is completely necessary. We don't know things that are a constant compile-time value that can't be changed...
So how do we use it?
Code:
ResourceType QuestResources::get_resource_type_by_name(
const std::string& resource_type_name
) {
int i = 0;
for (const auto& kvp: resource_type_names) {
if (kvp.second == resource_type_name) {
return kvp.first;
}
++i;
}
Debug::die(std::string("Unknown resource type: ") + resource_type_name);
return ResourceType();
}
Great. Lets do a linear look-up a value that never changes from a string that's constant, and then let's add debugging info. Hey, you forgot exception handling dood!
It's a good that we didn't just make an enum instead of using proper OOP c++.
Well I stopped here after looking in only two files: The Quest*.cpp files. This post is going to be ridiculously large if I do more than that.
A quick question: How do tiles work?
http://www.solarus-games.org/wp-cont...r-1024x619.png
It looks like tilesets are just bitmaps and it doesn't really differentiate between the two. If that's the case what happens if you have a 100 tile animated tile?
I looked in Tileset.h but then holy fuck...
Code:
const std::string id; /**< id of the tileset */
std::map<std::string, std::unique_ptr<TilePattern>>
tile_patterns;
I haven't even checked the rendering at all, but it looks like it was all software rendered then switched to SDL2 but kept the same interface and soft styled drawing. I did a search of the sources and there is nothing in the way of "Batch" in any keywords. This throws up some major red flags.
My point stands: If you want to use the Solarus data and code, it might be a good idea to look seriously at the Solarus data and code and assess it from the inside out.
tl;dr - I hate c++.
