Bracketed `<name>` entries in `gamelist.xml` have spacing removed between them

[retrobit]

Describe the bug

When brackets are generated for the title (provided from cache, either from the original filename or from a scraper source), there is missing spacing.

For example Adian no Tsue (Japan) (Beta) (1986-10-29) becomes Adian no Tsue (Japan)(Beta)(1986-10-29)

To Reproduce

1. Scrape with screenscraper source
2. Generate a gamelist.xml
3. Browse or open the gamelist.xml directly to see the difference between the filename in <path> and the <name> entry

Expected behavior

Bracketed entries are in the title (as is default behavior) but they are separated by spaces

Special circumstances

I am using ScreenScraper as my scraping source; need to see how brackets are scraped/cached, but this may just be how ScreenScraper provides these details. If so, then this is not an issue with Skyscraper.

This happens either with or without forceFilename=true from the configi.ini or --flags forcefilename on the CLI - documented behavior is that bracketed entries might "double-up" between filename and scraping source, but this is a different issue

Terminal output

N/A

Technical information

• Skyscraper version: 3.12.0 (latest at time of writing)
  • RetroPie version: 4.8.8 (latest at time of writing)

Additional context

N/A

[retrobit]

Willing to work on this and create a PR, but wanted to make sure it was seen as a defect and would be welcome first

[Gemba]

Hello again, thanks for the detailed report.

forceFilename if set, yields the basename of the filename with bracket/parenthesis content removed. I need to review the documentation for that.

To have the bracket/parenthesis content added the brackets flag must be set, as you noted. This adds first the parenthesis and then the brackets content without spaces in between. This behaviour is not changed from Lars' implementation. I can remember having unit-tests done before refactoring this a few months ago.

Anyhow, if you want to do a feature PR (and not a bugfix PR) you are more than welcomed. As this is a feature I would suggest to have an additional flag in the [main] config section which, when set true, adds a space between each() and []. If it is unset the behaviour would be as currently implemented (to maintain backward compatibility).

(Or -which might be over-shooting- an option to replace the inner )( of consecutive parenthesis with something user provided e.g. ,. But this should then also be configurable for the ][.)

[retrobit]

Thanks for implementing this - I was not able to find time to work on it