Add filter file support when creating Resource Groups. by CCPCookies · Pull Request #18 · carbonengine/resources

CCPCookies · 2026-03-04T15:33:19Z

Changes

Filter file rules loading through legacy INI file format support added.
Filter logic matched from resfileserver and eve-resparser.
Documentation of filter file format added.
Documentation covering filter logic added.
Test coverage added for all known filter include scenarios.
Added new global filter rule which is useful for excluding .red files.
Filter logic for 'resfile' field not covered as it is covered by respaths logic.
New publicly exposed library function added to create Resource Groups with filtering.
Logic added to library function to allow skipping empty search directories.
Logic added to library to allow ascertaining compression size from remote filesystem.
Test coverage for library function added.
CLI extended to add Resource Group creation with filters.
Test coverage for CLI operation added.

Version

Rather than changing 'CreateFromDirectory' in library and 'create-group' in CLI. A new function and operation was added to create Resource Groups using filters. This was to keep the API stable as there are now other parties working with the origional commands. Version minor was bumped.

Extra

Filter ini file parsing logic is from PR16

Changes ------- * Filter file rules loading through legacy INI file format support added. * Filter logic matched from resfileserver and eve-resparser. * Documentation of filter file format added. * Documentation covering filter logic added. * Test coverage added for all known filter include scenarios. * Added new global filter rule which is useful for excluding .red files. * Filter logic for 'resfile' field not covered as it is covered by respaths logic. * New publicly exposed library function added to create Resource Groups with filtering. * Logic added to library function to allow skipping empty search directories. * Logic added to library to allow ascertaining compression size from remote filesystem. * Test coverage for library function added. * CLI extended to add Resource Group creation with filters. * Test coverage for CLI operation added. Version ------- Rather than changing 'CreateFromDirectory' in library and 'create-group' in CLI. A new function and operation was added to create Resource Groups using filters. This was to keep the API stable as there are now other parties working with the origional commands. Version minor was bumped. Extra ----- Filter ini file parsing logic is from PR16

tools/src/Downloader.cpp

ccp-chargeback · 2026-03-05T14:18:52Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+
+#include "CreateResourceGroupFromFilterCliOperation.h"
+
+#include <string>


Minor, order of includes as per coding guidelines
https://didactic-adventure-egnoryz.pages.github.io/cpp_coding_guidelines.html#order-of-includes

ccp-chargeback · 2026-03-05T14:35:35Z

tools/include/FilterFileReader.h

+
+struct FilterFile
+{
+	std::unordered_map<std::string, std::shared_ptr<Prefix>> prefixes;


NOTE when you iterate over this std::unordered_map, you will not get the items returned in the insertion order.
I "think" you may want it to return items in insertion order as part of the ResourceFilter::SetFromFilterFileData() function, that is calling m_prefixPaths.push_back(), which is NOT guaranteed as currently implemented.

Yes you are right, nice catch

I will also add a test to enforce the order importance

ccp-chargeback · 2026-03-05T14:57:48Z

include/ResourceGroup.h

 	/// @note No file filtering supported
 	Result CreateFromDirectory( const CreateResourceGroupFromDirectoryParams& params );

+    /// @brief Creates a ResourceGroup from a supplied filter files.


Typo.
... from supplied filter files. (remove the a)
or
... from a supplied filter file. (change to singular, remove s)

ccp-chargeback · 2026-03-05T15:00:35Z

tools/src/FilterFileReader.cpp

+
+	ParseIncludeExcludeRules( globalFiltersStr, fileData.includeRules, fileData.excludeRules );
+
+	// Get section infomration


Typo:
// Get section information

ccp-chargeback · 2026-03-05T15:09:37Z

tools/src/FilterFileReader.cpp

+	ParseIncludeExcludeRules( globalFiltersStr, fileData.includeRules, fileData.excludeRules );
+
+	// Get section infomration
+	for( const auto& sectionName : reader.Sections() )


NOTE:
The INIReader will return Sections in alphabetical order, not the order they appear in the .ini file.
If you want to keep the sections in the order as defined in the file, you will have to read the file manually and find all the sections and then iterate over that list (instead of reader.Sections())

I.e. change it to do:
std::vectorstd::string sectionsInOrder = ManuallyReadIniFileSectionsInOrderExcludingDefault();
for( const auto& sectionName : sectionsInOrder)

Yes, I remember you saying this. I don't see a scenario where order of sections matter.

I would also not want to be doing any ini parsing manually, this needs to be supplied by a library. The lib you found so far appears to be sort of ok. Another annoying thing it does is removes all the casing from the section names, not huge but I'd prefer it didn't.

I agree. It's annoying that it lowercases everything.
It also cuts each sectionName to the first either 48 or 50 characters (can't remember which)
But this ini reader was the best fit, because it:

had vcpkg support

emulated the original python ini file implementation.

As for if order of sections matter.
What about if the same respath and prefix are present in two different [namedSections], but one of them has an [someFilter] (include) where the other has the same ![someFilter] (exclude filter)?
What happens there?

This will match as it would with previous system

ccp-chargeback · 2026-03-05T15:45:54Z

doc/source/DesignDocuments/fileFiltering.rst

+
+1. Globally in the ``[DEFAULT]`` section using ``filter =`` .
+2. Section locally in sections using ``filter =`` .
+3. Semi locally to respath adding include/exclude rules to each path ``respaths = prefix1:/* [ include ] ![ exclude ]``


This is correct,
but it renders strange when viewed in a browser (splits the line).
It would be better if the "respaths = prefix1:/* [ include ] ![ exclude ]" would be put on the line below.

ccp-chargeback · 2026-03-05T15:54:34Z

doc/source/DesignDocuments/fileFiltering.rst

+
+2. Section local filters are combined with any filters specified in global filters.
+
+3. respaths filters combine with both global and section filters and importantly these add for all subsequent paths. This is explained more in the following examples.


Isn't this supposed to be:
"...and importantly these add for all subsequent paths WITHIN THE SELECTED SECTION."
I.e. filter defined in [SectionA] will not also be applied to all entries in [SectionB] onwards.

ccp-chargeback · 2026-03-05T15:58:06Z

doc/source/DesignDocuments/fileFiltering.rst

+Two paths will be tested for inclusion: 
+
+``#3`` will use ``respaths = prefix1:/*`` and combine global and section local patterns ``include1`` and ``include2``. This will match the following from the source files:
+1. ``include1.txt``


Add an empty line before the "1. include1.txt", for it to render correctly in a browser.

ccp-chargeback · 2026-03-05T15:58:27Z

doc/source/DesignDocuments/fileFiltering.rst

+2. ``include2.txt``
+
+``#4`` will use ``respaths = prefix1:/* [ include3 ]`` which will extend the section local patterns to include ``include3``. This will match the following source files:
+1. ``include1.txt``


Add empty line, so it renders correctly in a browser.

ccp-chargeback · 2026-03-05T15:58:44Z

doc/source/DesignDocuments/fileFiltering.rst

+3. ``include3.txt``
+
+``#5`` will use ``respaths = prefix2:/*`` and doesn't sepecify any include rules. It will apply the include rules that have been constructed for the section at this point ``include1``, ``include2`` and ``include3``. This may be suprising. So this will match the following source files:
+1. ``Path/include3.txt``


Add empty line to it renders correctly in browser.

ccp-chargeback · 2026-03-05T16:06:27Z

doc/source/DesignDocuments/fileFiltering.rst

+
+    [exampleSection]
+        filter = [ include2 ]                   # 2. Section local include
+        respaths = prefix1:/*                   # 3. respath1


NOTE this is not how multiple (multi-line) respaths are defined in existing res.ini files.
The correct example would look like:

[resCharacterMisc] respaths = res:/Graphics/Character/Global/... res:/Graphics/Character/Female/Skeleton/... res:/Graphics/Character/Female/* res:/Graphics/Character/Male/Skeleton/... res:/Graphics/Character/Male/* res:/Graphics/Character/Unique/...

Where the multi-line entries are within the SAME "respaths" attribute.

Ah yeah, I'll change the documentation. The actual tests don't do this, this is just documentation.

ccp-chargeback · 2026-03-05T16:27:08Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+#include <unordered_set>
+
+CreateResourceGroupFromFilterCliOperation::CreateResourceGroupFromFilterCliOperation() :
+	CliOperation( "create-group-from-filter", "Create a Resource Group from a filter files." ),


mixing singular and plural

ccp-chargeback · 2026-03-06T12:04:25Z

src/ResourceGroupImpl.cpp

 #include <Md5ChecksumStream.h>
 #include <GzipCompressionStream.h>
+#include <cctype>
 #include "ResourceInfo/PatchResourceGroupInfo.h"


Look at include grouping and ordering from:
https://didactic-adventure-egnoryz.pages.github.io/cpp_coding_guidelines.html#order-of-includes

ccp-chargeback · 2026-03-06T12:07:09Z

src/ResourceGroupImpl.cpp

+		return Result{ ResultType::FAILED_TO_INITIALIZE_RESOURCE_FILTER, errorMsg };
+	}
+
+	statusSettings.Update( StatusProgressType::PERCENTAGE, 0, 5, "Loading filter files" );


This status line has 0, 5 like the line for "Create resource group from filters".
Should the numbers be updated (is this a copy-paste error)?

ccp-chargeback · 2026-03-06T12:21:21Z

src/ResourceGroupImpl.cpp

+
+			if( inputDirectoryStatus.RequiresStatusUpdates() )
+			{
+				float step = static_cast<float>( 100.0 / searchPaths.size() );


The step variable is going to be the same in every iteration of the loop.
Can be calculated outside fo the for loop.

Nah, this way that computation is skipped if not verbose, so we don't calculate something we don't need when not caring about the output.

ccp-chargeback · 2026-03-06T12:27:30Z

src/ResourceGroupImpl.cpp

+				}
+				else
+				{
+					return Result{ ResultType::INPUT_DIRECTORY_DOESNT_EXIST, inputDirectory.string() };


I'm confused.
Why would you ever not want to skip non existent input directories?

Is it only for testing/debug purposes?

No this is due to real world usecase of reduced-resources.

It only syncs files that changed, so in theory it might (and usually does) not a single file in a search directory. If it's not synced then there is no directory. But this is not a fail case.

ccp-chargeback · 2026-03-06T14:41:47Z

src/ResourceGroupImpl.cpp

+
+						ss << "Processing file: "
+						   << filePathRelativeToInputDirectory.string()
+						   << ", Match filter: "


"Match filter:" vs matchSection
I guess this is supposed to be "Match Section:" or "Match Section Id:"
Unless you also reference return the "current include/exclude filter from the CheckPath() function and return that as well. Might be useful for debugging.

I actually was thinking to return the current line number for the path rule so that you can see really well. But not required any further information so didn't bother to skip some computation time.

But the wording is still wrong though, right?
This is supposed to be "section", not "filter", right?

Sorry, yeah, i've changed it to matched filter section

ccp-chargeback · 2026-03-06T14:47:45Z

src/ResourceGroupImpl.cpp

+							resourceParams.binaryOperation = ResourceTools::CalculateBinaryOperation( entry.path() );
+						}
+
+						Location l;


Can you change the name of the variable to be more descriptive?

src/ResourceGroupImpl.cpp

ccp-chargeback · 2026-03-06T15:21:45Z

tools/include/FilterFileReader.h

+
+#include <filesystem>
+#include <vector>
+#include <unordered_map>


Imports in alphabetical order

ccp-chargeback · 2026-03-06T15:27:15Z

tools/include/ResourceFilter.h

+
+struct FilterPath
+{
+	std::string sectionId;


In other structs / classes you've put an empty line between items.
Missing in this struct.

tools/include/ResourceFilter.h

ccp-chargeback · 2026-03-09T11:14:47Z

tests/src/ResourceToolsFilterTest.cpp

+
+	std::filesystem::path invalidPath = "File";
+
+	ASSERT_FALSE( resourceFilter.CheckPath( invalidPath ) );


Also check if something is valid

ccp-chargeback · 2026-03-09T11:17:53Z

tests/src/ResourceToolsFilterTest.cpp

+
+	std::filesystem::path prefix2InvalidPath = "Path2/File";
+
+	ASSERT_FALSE( resourceFilter.CheckPath( prefix2InvalidPath ) );


You might want to check for "SomethingValid" on both prefixes as well.

ccp-chargeback · 2026-03-09T11:18:30Z

tests/src/ResourceToolsFilterTest.cpp

+
+	std::filesystem::path prefix2ValidPath = "Path2/File";
+
+	ASSERT_TRUE( resourceFilter.CheckPath( prefix2ValidPath ) );


What about invalid checks in this test?

ccp-chargeback · 2026-03-09T11:19:49Z

tests/src/ResourceToolsFilterTest.cpp

+
+	std::filesystem::path prefix2InvalidPath = "Path2/File";
+
+	ASSERT_FALSE( resourceFilter.CheckPath( prefix2InvalidPath ) );


Add checks for "SomethingValid" on both prefixes as well.

ccp-chargeback · 2026-03-09T11:24:24Z

tests/src/ResourceToolsFilterTest.cpp

+
+	std::filesystem::path invalidPath = "File";
+
+	ASSERT_FALSE( resourceFilter.CheckPath( invalidPath ) );


What happens if you check for "SomethingElse", will it match or not?

This change has been tested against all real branch data with no obvious differences from the previous system it is to replace. * Get index filter mapping from yaml file passed to CLI * Reworked filter library processing to process many groups at once * Filter logic now forced to lowercase to match previous tool * Addressed previous draft PR feedback

ccp-chargeback · 2026-03-12T12:11:41Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+    // Load filter settings from file
+	std::filesystem::path basePathToFilterFiles = m_argumentParser->get<std::string>( m_filterFileBasePathId );
+
+    std::filesystem::path basePathRespourceFiles = m_argumentParser->get<std::string>( m_resourceFileBasePathId );


I guess this is the French version "Respource" ?

ccp-chargeback · 2026-03-12T12:15:20Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+	}
+
+    // Load filter settings from file
+	std::filesystem::path basePathToFilterFiles = m_argumentParser->get<std::string>( m_filterFileBasePathId );


Mostly commenting for myself:
So we have two basepaths:

FilterFiles => used to resolve paths defined in the .ini files

ResourceFiles => is that the basepath of the incoming files (from VCS), or is this something else?

Actually we have 3

prefixMapBasePath as well

I now know what each of them does, good documentation in the AddArgument() functions, but I started looking at the header file that was missing it, causing confusion.

ccp-chargeback · 2026-03-12T15:56:06Z

src/ResourceInfo/ResourceInfo.cpp

+		m_compressedSize = resourceInfo.m_compressedSize;
+	}
+
+	m_uncompressedSize = resourceInfo.m_uncompressedSize;


Why are only some of the DocumentParameter member variables doing a .HasValue() checks?
All of them contain an underlying m_value of a std::optional type

ccp-chargeback · 2026-03-12T17:05:07Z

cli/src/CreateResourceGroupFromFilterCliOperation.h

+
+    std::string m_filterIndexMappingFileId;
+
+    std::string m_filterFileBasePathId;


It would be so good to have comments on what each of those do.
I'm having a bit of a brain aneurysm, wrapping my head around the differences between the:

filterFileBasePathId

resourceFileBasePathId

prefixMapBasePathId

ccp-chargeback · 2026-03-12T17:07:45Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+
+    AddArgument( m_filterFileBasePathId, "Base path to filter files.", true, true, "" );
+
+    AddArgument( m_resourceFileBasePathId, "Base path for output resource files.", false, true, "" );


Could this be called outputResourceFileBasePathId or resourceOutputFileBasePathId instead?

ccp-chargeback · 2026-03-12T17:12:08Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+
+	AddArgument( m_exportResourcesDestinationPathId, "Represents the base path where the exported resources will be saved. Requires --export-resources", false, false, defaultImportParams.exportSettings.destinationSettings.basePath.string() );
+
+	AddArgument( m_prefixMapBasepathId, "Base directory for prefix mappings defined in filter files.", false, false, "" );


Shouldn't this be a required=true

Because you would never call CreateResourceFromFILTER unless you're passing in filter .ini file(s) and then the basepath for the prefix should be required, right?

ccp-chargeback · 2026-03-12T17:27:43Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+	}
+
+	std::cout << "Resource filter index mapping file: " << filterIndexMappingFilePath << std::endl;
+


Should it ouput the contents of the mapping file here as well?

ccp-chargeback · 2026-03-12T17:38:07Z

cli/src/CreateResourceGroupFromFilterCliOperation.cpp

+	}
+
+	auto resourceIter = resourceGroups.begin();
+	for( auto exportIter = exportParams.begin(); exportIter != exportParams.end(); exportIter++, resourceIter++ )


This section would benefit from a short comment for clarity.

ccp-chargeback · 2026-03-12T17:42:01Z

doc/source/DesignDocuments/fileFiltering.rst

+
+    [exampleSection]                                    # [OPTIONAL] Filter section
+        filter = [ include ] ![ exclude ]               # [OPTIONAL] Section local include and exclude patterns
+        respaths = prefix1:/*                           # [OPTIONAL] Path pattern


Respaths is a single attributeName but multi-line value.

ccp-chargeback · 2026-03-12T17:45:16Z

include/ResourceGroup.h


    ResourceDestinationSettings exportResourcesDestinationSettings = { CarbonResources::ResourceDestinationType::LOCAL_CDN, "ExportedResources" };
+
+    uintmax_t fileStreamChunkSize = 20971520;


Magic number, replace with a NAMED_CONSTANT

ccp-chargeback · 2026-03-12T17:46:06Z

include/ResourceGroup.h

+    */
+struct CreateResourceGroupFromFilterParams
+{
+	uintmax_t fileStreamChunkSize = 20971520;


Magic number again

ccp-chargeback · 2026-03-13T09:12:58Z

tools/src/FilterIndexMappingFile.cpp

+
+#include <yaml-cpp/yaml.h>
+
+FilterIndexMappingFile::FilterIndexMappingFile()


Empty constructor and destructor.
Make it "default" in the header file
FilterIndexMappingFile() = default;
~FilterIndexMappingFile() = default;

ccp-chargeback · 2026-03-13T09:13:15Z

tools/src/FilterIndexMappingFile.cpp

+bool FilterIndexMappingFile::LoadFromFile( const std::filesystem::path& path )
+{
+
+	// TODO add returns for invalid file


ccp-chargeback · 2026-03-13T09:23:37Z

src/ResourceGroupImpl.cpp

+
+    bool ignoreCase = false;
+
+    if (documentVersion == VersionInternal(0, 0, 0))


It would be good to know why this is needed on csv files

Maybe created consts for 0.0.0 and 0.1.0
Similar to S_DOCUMENT_VERSION

ccp-chargeback · 2026-03-13T09:36:53Z

src/ResourceGroupImpl.cpp

+		int i = 0;
+		for( auto searchPath : searchPaths )
+		{
+			std::filesystem::path inputDirectory = params.filterSettings.prefixMapBasePath / searchPath;


Couldn't this become just a single filename?
Asking because there are recursivDirectoryIterator for loops further down in this function.

Wondering about speed improvements in those cases.

ccp-chargeback · 2026-03-13T09:42:29Z

src/ResourceGroupImpl.cpp

+
+					std::filesystem::path entryPath = entry.path();
+
+					std::string entryPathString;


This entryPathString is never appended to.
Where it is being used (to print out some status info) is only within RequireStatusUpdates() if statements blocks of their own.
I think you could just remove it and just use entryPath.string() instead in those cases.

ccp-chargeback · 2026-03-13T09:49:10Z

src/ResourceGroupImpl.cpp

+								continue;
+							}
+
+							if( filter->CheckPath( normalisedRelativePath, matchSection, matchPath ) )


If ignoreCase==true and normalisedRelativePath has been made lower case.
Doesn't the filter->CheckPath need to make sure it does lowercase comparison as well?

ccp-chargeback · 2026-03-13T09:55:21Z

src/ResourceGroupImpl.h


 	Result RemoveResource( ResourceInfo& relativePath );

+    bool ResourceExists( ResourceInfo& resource );


This function is never used anywhere (as far as I can tell)

ccp-chargeback · 2026-03-13T09:58:09Z

tools/include/ResourceFilter.h

+class ResourceFilter
+{
+public:
+	ResourceFilter();


This can be changed to be:
ResourceFilter() = default;

Same for destructor, as there is no implementation of either in the .cpp file (and it can be removed from those files)

CCPCookies added 3 commits March 4, 2026 15:15

Remove semicolon that was that was causing build failure on macOS

e90228b

Remove redundant whitespace from cli argument in test

d429126

CCPCookies requested a review from ccp-chargeback March 5, 2026 10:03

ccp-chargeback reviewed Mar 5, 2026

View reviewed changes

tools/src/Downloader.cpp Show resolved Hide resolved

ccp-chargeback reviewed Mar 5, 2026

View reviewed changes

ccp-chargeback reviewed Mar 6, 2026

View reviewed changes

src/ResourceGroupImpl.cpp Outdated Show resolved Hide resolved

ccp-chargeback reviewed Mar 6, 2026

View reviewed changes

src/ResourceGroupImpl.cpp Show resolved Hide resolved

ccp-chargeback reviewed Mar 6, 2026

View reviewed changes

src/ResourceGroupImpl.cpp Outdated Show resolved Hide resolved

ccp-chargeback reviewed Mar 6, 2026

View reviewed changes

src/ResourceGroupImpl.cpp Outdated Show resolved Hide resolved

ccp-chargeback reviewed Mar 6, 2026

View reviewed changes

tools/include/ResourceFilter.h Show resolved Hide resolved

ccp-chargeback reviewed Mar 9, 2026

View reviewed changes

CCPCookies added 3 commits March 11, 2026 15:12

Remove unused reference that was failing macOS build

42c40f8

Remove namespace that was causing macOS build to fail

dcbef83

CCPCookies marked this pull request as ready for review March 11, 2026 16:08

CCPCookies requested a review from ccp-chargeback March 11, 2026 16:08

Remove goto that no longer makes sense

d22ce75

ccp-chargeback reviewed Mar 12, 2026

View reviewed changes

ccp-chargeback reviewed Mar 13, 2026

View reviewed changes


		#include "CreateResourceGroupFromFilterCliOperation.h"

		#include <string>


		ParseIncludeExcludeRules( globalFiltersStr, fileData.includeRules, fileData.excludeRules );

		// Get section infomration


		2. Section local filters are combined with any filters specified in global filters.

		3. respaths filters combine with both global and section filters and importantly these add for all subsequent paths. This is explained more in the following examples.


		std::filesystem::path invalidPath = "File";

		ASSERT_FALSE( resourceFilter.CheckPath( invalidPath ) );


		std::filesystem::path prefix2InvalidPath = "Path2/File";

		ASSERT_FALSE( resourceFilter.CheckPath( prefix2InvalidPath ) );


		std::filesystem::path prefix2ValidPath = "Path2/File";

		ASSERT_TRUE( resourceFilter.CheckPath( prefix2ValidPath ) );


		std::string m_filterIndexMappingFileId;

		std::string m_filterFileBasePathId;


		AddArgument( m_filterFileBasePathId, "Base path to filter files.", true, true, "" );

		AddArgument( m_resourceFileBasePathId, "Base path for output resource files.", false, true, "" );


		AddArgument( m_exportResourcesDestinationPathId, "Represents the base path where the exported resources will be saved. Requires --export-resources", false, false, defaultImportParams.exportSettings.destinationSettings.basePath.string() );

		AddArgument( m_prefixMapBasepathId, "Base directory for prefix mappings defined in filter files.", false, false, "" );

		}

		std::cout << "Resource filter index mapping file: " << filterIndexMappingFilePath << std::endl;


		ResourceDestinationSettings exportResourcesDestinationSettings = { CarbonResources::ResourceDestinationType::LOCAL_CDN, "ExportedResources" };

		uintmax_t fileStreamChunkSize = 20971520;


		#include <yaml-cpp/yaml.h>

		FilterIndexMappingFile::FilterIndexMappingFile()


		bool ignoreCase = false;

		if (documentVersion == VersionInternal(0, 0, 0))


		std::filesystem::path entryPath = entry.path();

		std::string entryPathString;


		Result RemoveResource( ResourceInfo& relativePath );

		bool ResourceExists( ResourceInfo& resource );

Conversation

CCPCookies commented Mar 4, 2026

Changes

Version

Extra

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!