TEST: Reformatted XML files

[Jenalya]

Since the formats of our XML files are/were rather different, some of them with a mixture of tabs and spaces, I looked into a way to automatically format them.
For those interested how: https://github.com/jtoelke/tmwa-client- ... a15aa624d9
Any suggestions and feedback on how to improve that is welcome.

No functionality should have changed. I put the reformatted XML files in the client-data updates of the testserver.
XML is involved in nearly everything, player, NPCs, equipment, particles, monsters, smileys...

As there is no new content, it might be a bit boring to test on the testserver, so if you have git, you could also clone or fetch from my fork on github: https://github.com/jtoelke/tmwa-client-data and start your client with the options

Code: Select all

mana -u -d /path/to/client-data

or

Code: Select all

manaplus -u -d /path/to/client-data

and play on the main server while using the updated files.

[o11c]

Hm, I used the 'xmlindent' program.

(Using xml to handle xml has always seemed an exercise in frustration)

Ideally you would use find -exec instead of a for loop, but then it would be difficult to except items.xml and npcs.xml - why are they?

[Jenalya]

Ideally you would use find -exec instead of a for loop, but then it would be difficult to except items.xml and npcs.xml - why are they?

In npcs.xml it puts the comments in a new line, which results in the commands being below the tag they belong to, which is a bit counterintuitive.
In items.xml there are a lot of entries with plenty attributes, which each get an extra line currently, but are put into a single one when using that xsl. I found there is an items.xsl in client-data, but using it with xsltproc didn't work, the output was rather weird.
Since the format of those files is alright (though a bit different than the rest now), I chose to just except them instead of trying to find out how to modify xsl files to make it work.

[Whistler]

Not sure if it's a psychological effect knowing the changes
or just me but playing on the main using your work seems to
slightly improve the game experience, more reactivity.
Nothing that far reacting unexpectedly or being affected as far
as the few hours i've been playing.

[o11c]

Ideally you would use find -exec instead of a for loop, but then it would be difficult to except items.xml and npcs.xml - why are they?

In npcs.xml it puts the comments in a new line, which results in the commands being below the tag they belong to, which is a bit counterintuitive.

The correct solution is to put the comment on the previous line. This could be done with a sed script, something like this (untested): s/^[[:space]]*\([^[:space:]]+\)[[:space:]]*\(<!--.*-->\)[[:space:]]*$/\2\n\1/

In items.xml there are a lot of entries with plenty attributes, which each get an extra line currently, but are put into a single one when using that xsl.

I think xmlindent might preserve them.

[baedamichi]

you could also clone or fetch from my fork on github: https://github.com/jtoelke/tmwa-client-data and start your client with the options

Code: Select all

mana -u -d /path/to/client-data

or

Code: Select all

manaplus -u -d /path/to/client-data

and play on the main server while using the updated files.

I did exatcly that. I've been playing using "your" files now for two days, and so far I couldn't note any differences, so it seems your reformatting doesn't cause any trouble. Of course, I can't say I talked to each and every NPC or tried each and every thing you can do in this game, but I've tried quite a lot, and had no trouble whatsoever. I'll keep on playing using your files Jenalya, if anything should come up I'll post here.

Just something I was wondering about, Jenalya's .xml files seem to be different from the "official" ones (those that came with the last automatic update). I came across this rather randomly, when trying to correct a mistake in one of the .xml files (Crusade Helmet; problem: character's hair could still be seen when standing facing to the left or walking to the right; that problem only occured while using Jenalya's files, seems to already have been fixed on the main server). Now I can't say I understand to a full extent how exactly an xml file works, but I understand enough to see that the xml files on the main server are different from Jenalya's files (and the difference is not limited to separators being tabs versus spaces). The difference I noted is that Jenalya's file, for example, has four lines below "animation direction=down" (action name = walk) whereas the "official" version has six.
Now my intution told me that when testing things you should always use the latest version. But being a "computer idiot", it is highly likely that my intution is wrong here.
Of course, I'm not asking any of the devs to explain to me how xml files work, their time is too precious to do that. All I'm asking for is a simple yes/no answer to the following question:
Does the difference I noted matter at all when testing whether a tab/space reformatting for consistency causes any trouble?

[Jenalya]

Now I can't say I understand to a full extent how exactly an xml file works, but I understand enough to see that the xml files on the main server are different from Jenalya's files (and the difference is not limited to separators being tabs versus spaces).

The changes I did shouldn't have changed anything but the format. I wonder which file you compared with, did you look into the local files that were downloaded from the client when logging in? And do you use manaplus? Since, as far as I know, 4144 provides some custom client-updates to support some features that manaplus has. When you log in there's a checkbox with 'Custom update host'. If you have that checked and compared with the dowloaded files, you were comparing with manaplus custom files.

@o11c: I'm going to try out your suggestions.

[baedamichi]

And do you use manaplus? Since, as far as I know, 4144 provides some custom client-updates to support some features that manaplus has.

Yes, I do. That was the reason. Thanks for making me wiser today.

[Jenalya]

Update: I pushed another commit in my repo to format items.xml. For those testing with a clone of my repo: use 'git pull' to get the latest version.

The correct solution is to put the comment on the previous line. This could be done with a sed script, something like this (untested): s/^[[:space]]*\([^[:space:]]+\)[[:space:]]*\(<!--.*-->\)[[:space:]]*$/\2\n\1/

This gives

bash: !--.*--: event not found

I tried using xmlindent instead of xsltproc in my script and found that it adds trailing whitespaces in empty lines, there are still some tabs in some files and in some files it added additional empty lines. I don't really get in which cases this happens, but since the script works fine with xsltproc, I'd prefer just to stick with that.
I used xmlindent on only items.xml and removed the trailing whitespaces that were added by resaving in my editor.

[o11c]

I hate history expansion.

This works for me:

Code: Select all

sed 's/^\(.*\)\(<!--.*-->\)\(.*\)$/\2\1\3/' -i npcs.xml

,
regardless of whether bash is "set -o histexpand" (enabled) or "set +o histexpand" (disabled). (short version: set -H, set +H)

Just a few manual fixups are needed for NPCs 251..255 since they cover multiple lines (You might actually want to do that before the sed; search for /^</npc>/ )

Now I remember, when using xmlindent I had to remove the trailing whitespace in a second stage, but that's easy enough in a script:

Code: Select all

sed 's/[[:space:]]*$//'
[code]

"there are still some tabs in some files" is a more serious problem, can you point me at a specific file?

I don't particularly care about which indenter is used, the only question is does it work?

[Jenalya]

Update: I pushed some more commits in my repo. For those testing with a clone of my repo: use 'git pull' to get the latest version.

This works for me:

Code: Select all

sed 's/^\(.*\)\(<!--.*-->\)\(.*\)$/\2\1\3/' -i npcs.xml

Alright, that worked. I applied that, did some manual edits to the comments of NPCs 251 to 265 (by the way, are you working with an up-to-date repo, since it's up to 265, not 255) and removed npcs.xml from being excepted in the script.

I also added two lines taking care of items.xml using xmlindent, so the script now looks like this:

Code: Select all

#!/bin/bash

cd ..

for f in $(find -name "*.xml")
do
    if [ ${f} != "./items.xml" ]; then
        xsltproc tools/indent.xsl ${f} > ${f}__
        mv ${f}__ ${f}
    fi
done
xmlindent items.xml > items.xml_
sed 's/[[:space:]]*$//' items.xml_ > items.xml

"there are still some tabs in some files" is a more serious problem, can you point me at a specific file?

It happens e.g. in emotes.xml, the issue with the extra empty lines happens in e.g. graphics/emotes/afraid.xml

I don't particularly care about which indenter is used, the only question is does it work?

After a few manual edits as e.g. moving the comments in npcs.xml, the latest version of the script should work to automatically format all xml files in client-data.

[baedamichi]

I pushed some more commits in my repo.

I've been testing using those until today, and experienced no trouble whatsoever, everything worked just as expected.

[Jenalya]

I squashed the commits from my fork and pushed them to the main repository. So now there's one commit adding the script and another one with the actual changes to the xml files.

Thanks for testing and suggestions!