Bug fix for Carcassonne display

Game development with Board Game Arena Studio
User avatar
shadowphiar
Posts: 124
Joined: 01 January 2017, 16:07

Bug fix for Carcassonne display

Post by shadowphiar »

I've been doing some digging into a display bug (#6090) which affects Carcassonne when run on Safari (either on Mac OS or iPhone/iPad). I believe I've identified the cause, but Carcassonne (developed by Board Game Arena) isn't a project on BGA Studio so I can't clone it to test a change to the javascript (I can see the minified version but can't run any changed code). Is there any way I could either get access to the original source project, or could someone have a look at the below and see if my hypothesis works?

If the view is zoomed in or out from its default size at all, the bug manifests as tiles which are offset by a few pixels in one direction. At small sizes (including the default view in portrait on my iPhone 6S) the error can be a significant proportion of the tile, so it gets confusing to see what's going on. The offset is always in the tile's vertical direction, but tiles can be displayed with 90 degree rotation, so the displacement on screen can be in any direction. (Also you may notice that tiles change displacement when the map scrolls even if the zoom stays the same - but only if the whole interface is being zoomed smaller for being drawn on small screens).

A tile looks something like this (I've simplified it a bit):

Code: Select all

        <div id="tile_35" class="bdtile" style="top: 127.54473388671876px; left: -127.54473388671876px; width: 127.54469794418385px; height: 127.54469794418385px;">
            <div id="tile_art_35" class="tile_art" style="background-size: 382.63409383255157px 10458.665231423076px; background-position: 0% -2200%;">
            </div>
        </div>
Note that the size of div tile_35 is a non-integer number of pixels. The background-position of tile_art_35 is specified as a percentage, of the size of this div - which should be the same as the size of a tile, but it looks like Safari is rounding to an integer number of pixels that the box actually occupies on the screen, and using that rounded value in the calculation of what part of the image should be displayed. Up to half-a-pixel difference, multiplied by percentages like 2200% (because there are a lot of tiles placed vertically in that image) adds up to an offset that is quite substantial.

NB: I'm not saying Safari isn't wrong to be doing this truncation, but even if someone gets them to fix it, then it will still only look right for BGA users who have updated to the latest and greatest Safari version, and it may be possible to work around it in Carcassonne's code without breaking things for other browsers.

I believe that adding the line:

Code: Select all

this.tile_size = (Math.round(this.tile_size * this.gameinterface_zoomFactor)) / this.gameinterface_zoomFactor;

just after tile_size is written (in onWheel(), onUnzoom(), and the constructor) will probably have the right effect.

This arranges that the javascript does its calculations based on a tile size which will end up being a whole number of pixels on the screen, so the precise number and the truncated number agree.
User avatar
sourisdudesert
Administrateur
Posts: 4630
Joined: 23 January 2010, 22:02

Re: Bug fix for Carcassonne display

Post by sourisdudesert »

Hi,

Thanks a lot. This is difficult for us to perform tests on Safari, so this is very helpful to have such an analysis.

I've build a patch to make sure the number of pixel is always an integer. It's going to be available in few minutes on Carcassonne.

Hope this will do the trick.
User avatar
shadowphiar
Posts: 124
Joined: 01 January 2017, 16:07

Re: Bug fix for Carcassonne display

Post by shadowphiar »

Hi,
Thanks for looking at this. I can see an improvement on medium or large displays now. However on very narrow display such as my phone in portrait view, there are still artefacts unfortunately. I think Safari is taking a truncated value of the final screen dimensions after taking account of the effect of 'style: zoom' in the surrounding div with id='page-content'. I think this means the code fix does need to take account of this.gameinterface_zoomFactor.
Hope that helps,
Andrew
User avatar
sourisdudesert
Administrateur
Posts: 4630
Joined: 23 January 2010, 22:02

Re: Bug fix for Carcassonne display

Post by sourisdudesert »

Hi

I've done another fix, with the exact code you posted above.

I cannot see any difference, but it it did the trick for you it would be great...
User avatar
shadowphiar
Posts: 124
Joined: 01 January 2017, 16:07

Re: Bug fix for Carcassonne display

Post by shadowphiar »

Excellent, thank you this is looking a big improvement. It is now essentially perfect on desktop and iPad.

For the iPhone there's one case I forgot, which is that if the viewport size changes in a way that affects gameinterface_zoomFactor, the same rounding would need to be done to tilesize again. (This means that rotating the phone from portrait to landscape can leave the tiles looking wrong, although that is fixed with a reload). Sorry for the iterations.

(You'll know best how to hook up to that event, but it looks like there's a function onScreenWidthChange() which is called after the new gameinterface_zoomFactor was calculated).
User avatar
sourisdudesert
Administrateur
Posts: 4630
Joined: 23 January 2010, 22:02

Re: Bug fix for Carcassonne display

Post by sourisdudesert »

This another fix has been done too.

Happy to see it working for Safari users :)
User avatar
shadowphiar
Posts: 124
Joined: 01 January 2017, 16:07

Re: Bug fix for Carcassonne display

Post by shadowphiar »

Great, thank you! This is working perfectly now as far as I can see.
User avatar
SmilingDevil
Posts: 22
Joined: 28 November 2020, 07:29

Re: Bug fix for Carcassonne display

Post by SmilingDevil »

Well, now the next issue is : https://boardgamearena.com/bug?id=87361
So far no one seems to have been able to look into it…

This bug shows immediately if I reload the page as shown on various screenshots in the bug report.


For your entertainment:
Nobody plays on apple! https://youtu.be/Z0QgBX4WFKQ #VivaLaDirtLeague
User avatar
shadowphiar
Posts: 124
Joined: 01 January 2017, 16:07

Re: Bug fix for Carcassonne display

Post by shadowphiar »

I've had a look at this - again, I'm not the developer of Carcassonne, I don't have access to the (unminified) source code and I can't test or deploy any fixes. That said, I think I have found the bug and a fix.

Surprisingly, the new display bug is unrelated to the one initially described in this thread. The symptoms are similar but slightly different (an incorrectly displayed tile is always offset by exactly half, and displays the same at all zoom levels.) It's weirder than I imagined, and concerns the way Safari is parsing invalid syntax in the "background-position" style attribute in the tile_art_{n}" divs. If you look at one in the inspector you'll see an html representation such as:

Code: Select all

<div id="tile_art_6" class="tile_art first_edition" style="background-position: 9.090909%; transform: rotate(180deg); width: 88px; height: 88px;"></div>
Note the background-position has a single percentage value. This means that background-position-y becomes 50% and this is not what was intended, it is supposed to be 0%. How did it get there?

Turns out there is a bug in jstpl_tile_on_map, where there is no semicolon at the end of the "style" definition inside id="tile_art_${tile_id}"

This seems to cause the second argument to background-position not to get evaluated. It is immediately missing from node.style.cssText, and becomes invisible in the DOM representation when the style gets rewritten when the rotate transform is applied.

For what it's worth I have reproduced this in a minimal example, https://jsfiddle.net/shadowphiar/skh5q6yz/. On new Safari you will see the result "background-position: 10%;" and on other browsers "background-position: 10% 0%;". Arguably this is a Safari bug – it's certainly an unexpected result - I'll report it on webkit bugzilla but since it concerns faulty markup I don't think it will get very far. Either way, if BGA adds this semicolon I believe everything should look right again on all the browsers.
User avatar
SmilingDevil
Posts: 22
Joined: 28 November 2020, 07:29

Re: Bug fix for Carcassonne display

Post by SmilingDevil »

Really not looking forward to the suggested update of 16.5.

Did you dare already?
Post Reply

Return to “Developers”