calref / cboe

Classic Blades of Exile
http://spiderwebforums.ipbhost.com/index.php?/forum/12-blades-of-exile/
Other
168 stars 41 forks source link

First round of fixes for XML errors, there may be more #274

Closed seisatsu closed 1 year ago

seisatsu commented 4 years ago

This should fix all errors relating to the led element missing a width attribute, as well as the "neg pos" issue in edit-party.xml. I'll keep pushing files as I fix them.

CelticMinstrel commented 4 years ago

Thanks for the pull request! I want to review each of these cases in more detail though, so I'll hold off on merging it until I have some more time. Setting width to 14 is definitely the correct answer in some of those cases, but I think there are at least a few where it makes more sense to take the width from an associated <text> and move its contents into the <led> (thus deleting the <text>). Might also require repositioning the <led> (probably using the <text>'s location).

CelticMinstrel commented 1 year ago

This pull request is obsolete now, as LEDs no longer require a width attribute if they have no content. That said, there's the argument that many of these LEDs should have content, at which point they would need to have a width as well.

(The other change in this PR, not related to width, was already fixed manually in master.)