Browse Source

Make it legal to define multiple sprites with the same name, the last one is used. Allow anonymous sprite sheets.

Michael Ragazzon 4 years ago
parent
commit
53a5048970
3 changed files with 29 additions and 81 deletions
  1. 3 4
      Include/RmlUi/Core/Spritesheet.h
  2. 23 70
      Source/Core/Spritesheet.cpp
  3. 3 7
      Source/Core/StyleSheetParser.cpp

+ 3 - 4
Include/RmlUi/Core/Spritesheet.h

@@ -58,13 +58,11 @@ struct Spritesheet {
 	int definition_line_number;
 	float display_scale; // The inverse of the 'resolution' spritesheet property.
 	Texture texture;
-	StringList sprite_names;
 
 	Spritesheet(const String& name, const String& image_source, const String& definition_source,
 		int definition_line_number, float display_scale, const Texture& texture);
 };
 
-using SpritesheetMap = SmallUnorderedMap<String, SharedPtr<const Spritesheet>>; // key: spritesheet name (as given in @spritesheet)
 using SpriteDefinitionList = Vector<Pair<String, Rectangle>>; // Sprite name and rectangle
 
 
@@ -87,10 +85,11 @@ public:
 	size_t NumSpriteSheets() const;
 	size_t NumSprites() const;
 
-	String ToString() const;
 
 private:
-	SpritesheetMap spritesheet_map;
+	using Spritesheets = Vector<SharedPtr<const Spritesheet>>;
+
+	Spritesheets spritesheets;
 	SpriteMap sprite_map;
 };
 

+ 23 - 70
Source/Core/Spritesheet.cpp

@@ -45,30 +45,26 @@ bool SpritesheetList::AddSpriteSheet(const String& name, const String& image_sou
 	Texture texture;
 	texture.Set(image_source, definition_source);
 
-	auto sprite_sheet = MakeShared<Spritesheet>(name, image_source, definition_source, definition_line_number, display_scale, texture);
-	auto result = spritesheet_map.emplace(name, sprite_sheet);
-	if (!result.second)
-	{
-		Log::Message(Log::LT_WARNING, "Spritesheet '%s' has the same name as another spritesheet, ignored. See %s:%d", name.c_str(), definition_source.c_str(), definition_line_number);
-		return false;
-	}
+	auto sprite_sheet_ptr = MakeShared<Spritesheet>(name, image_source, definition_source, definition_line_number, display_scale, texture);
+	Spritesheet* sprite_sheet = sprite_sheet_ptr.get();
+	spritesheets.push_back(std::move(sprite_sheet_ptr));
 
-	StringList& sprite_names = sprite_sheet->sprite_names;
+	sprite_map.reserve(sprite_map.size() + sprite_definitions.size());
 
-	// Insert all the sprites with names not already defined in the global sprite list.
+	// Insert all the sprites, overwriting any existing sprites already defined in the current media block scope.
 	for (auto& sprite_definition : sprite_definitions)
 	{
 		const String& sprite_name = sprite_definition.first;
 		const Rectangle& sprite_rectangle = sprite_definition.second;
-		auto sprite_result = sprite_map.emplace(sprite_name, Sprite{ sprite_rectangle, sprite_sheet.get() });
-		if (sprite_result.second)
-		{
-			sprite_names.push_back(sprite_name);
-		}
-		else
+
+		Sprite& new_sprite = sprite_map[sprite_name];
+		if (new_sprite.sprite_sheet)
 		{
-			Log::Message(Log::LT_WARNING, "Sprite '%s' has the same name as an existing sprite, skipped. See %s:%d", sprite_name.c_str(), definition_source.c_str(), definition_line_number);
+			Log::Message(Log::LT_WARNING, "Sprite '%s' was overwritten due to duplicate names at the same block scope. Declared at %s:%d and %s:%d",
+				sprite_name.c_str(), new_sprite.sprite_sheet->definition_source.c_str(), new_sprite.sprite_sheet->definition_line_number, definition_source.c_str(), definition_line_number);
 		}
+
+		new_sprite = Sprite{ sprite_rectangle, sprite_sheet };
 	}
 
 	return true;
@@ -87,53 +83,28 @@ const Sprite* SpritesheetList::GetSprite(const String& name) const
 
 void SpritesheetList::Merge(const SpritesheetList& other)
 {
-	for (auto& pair : other.spritesheet_map)
+	spritesheets.insert(spritesheets.end(), other.spritesheets.begin(), other.spritesheets.end());
+	sprite_map.reserve(sprite_map.size() + other.sprite_map.size());
+
+	for (auto& pair : other.sprite_map)
 	{
-		auto sheet_result = spritesheet_map.emplace(pair);
-		const String& sheet_name = sheet_result.first->first;
-		const Spritesheet& sheet = *sheet_result.first->second;
-		bool sheet_inserted = sheet_result.second;
-		
-		if (sheet_inserted)
-		{
-			// The sprite sheet was succesfully added, now try to add each sprite to the global list.
-			for (const String& sprite_name  : sheet.sprite_names)
-			{
-				// Lookup the sprite in the other map.
-				auto it_sprite = other.sprite_map.find(sprite_name);
-				if (it_sprite != other.sprite_map.end())
-				{
-					// Add the sprite into our map. Each sprite name must be unique.
-					auto sprite_result = sprite_map.emplace(sprite_name, it_sprite->second);
-					bool inserted = sprite_result.second;
-
-					if (!inserted)
-					{
-						Log::Message(Log::LT_WARNING, "Duplicate sprite name '%s' found while merging style sheets, defined in %s:%d.", sprite_name.c_str(), sheet.definition_source.c_str(), sheet.definition_line_number);
-					}
-				}
-				else
-				{
-					RMLUI_ERRORMSG("Something went wrong while merging style sheets.");
-				}
-			}
-		}
-		else
-		{
-			Log::Message(Log::LT_WARNING, "Duplicate sprite sheet name '%s' found while merging style sheets, defined in %s:%d.", sheet_name.c_str(), sheet.definition_source.c_str(), sheet.definition_line_number);
-		}
+		const String& sprite_name = pair.first;
+		const Sprite& sprite = pair.second;
+
+		// Add the sprite into our map. If a sprite with the same name exists, it will be overwritten by the other sprite.
+		sprite_map[sprite_name] = sprite;
 	}
 }
 
 void SpritesheetList::Reserve(size_t size_sprite_sheets, size_t size_sprites) 
 { 
-	spritesheet_map.reserve(size_sprite_sheets);
+	spritesheets.reserve(size_sprite_sheets);
 	sprite_map.reserve(size_sprites);
 }
 
 size_t SpritesheetList::NumSpriteSheets() const 
 {
-	return spritesheet_map.size();
+	return spritesheets.size();
 }
 
 size_t SpritesheetList::NumSprites() const 
@@ -141,22 +112,4 @@ size_t SpritesheetList::NumSprites() const
 	return sprite_map.size();
 }
 
-String SpritesheetList::ToString() const
-{
-	String result = CreateString(100, "#SpriteSheets: %zu\n", spritesheet_map.size());
-
-	for (auto& sheet : spritesheet_map)
-	{
-		result += CreateString(100, "  Sheet '%s'.   #Sprites %zu.\n", sheet.first.c_str(), sheet.second->sprite_names.size());
-	}
-
-	result += CreateString(100, "\n#Sprites: %zu\n", sprite_map.size());
-	for (auto& sprite : sprite_map)
-	{
-		result += CreateString(100, "  In '%s': %s\n", sprite.second.sprite_sheet->name.c_str(), sprite.first.c_str());
-	}
-
-	return result;
-}
-
 } // namespace Rml

+ 3 - 7
Source/Core/StyleSheetParser.cpp

@@ -599,7 +599,7 @@ int StyleSheetParser::Parse(MediaBlockList& style_sheets, Stream* _stream, int b
 						current_block = {PropertyDictionary{}, UniquePtr<StyleSheet>(new StyleSheet())};
 					}
 
-					String at_rule_identifier = pre_token_str.substr(0, pre_token_str.find(' '));
+					const String at_rule_identifier = StringUtilities::StripWhitespace(pre_token_str.substr(0, pre_token_str.find(' ')));
 					at_rule_name = StringUtilities::StripWhitespace(pre_token_str.substr(at_rule_identifier.size()));
 
 					if (at_rule_identifier == "keyframes")
@@ -623,13 +623,9 @@ int StyleSheetParser::Parse(MediaBlockList& style_sheets, Stream* _stream, int b
 						const SpriteDefinitionList& sprite_definitions = spritesheet_property_parser->GetSpriteDefinitions();
 						const float image_resolution_factor = spritesheet_property_parser->GetImageResolutionFactor();
 						
-						if (at_rule_name.empty())
+						if (sprite_definitions.empty())
 						{
-							Log::Message(Log::LT_WARNING, "No name given for @spritesheet at %s:%d", stream_file_name.c_str(), line_number);
-						}
-						else if (sprite_definitions.empty())
-						{
-							Log::Message(Log::LT_WARNING, "Spritesheet with name '%s' has no sprites defined, ignored. At %s:%d", at_rule_name.c_str(), stream_file_name.c_str(), line_number);
+							Log::Message(Log::LT_WARNING, "Spritesheet '%s' has no sprites defined, ignored. At %s:%d", at_rule_name.c_str(), stream_file_name.c_str(), line_number);
 						}
 						else if (image_source.empty())
 						{