search

lichess-org / flutter-chessground

Chessboard package for flutter
https://pub.dev/packages/chessground
GNU General Public License v3.0
44 stars 27 forks source link

feat: allow scaling arrows #38

Closed tom-anders closed 2 months ago

tom-anders commented 2 months ago

I'd need this for implementing different arrow sizes in the mobile app, based on the engine evaluation of each move.

This is a really naive approach of course, but it will probably be alright for starters? We might want to apply a different scaling to the arrow body vs its tip in the future (Looks like lichess web UI does that as well).

veloce commented 2 months ago

Yes the scale should apply to the arrow body and tip at the same time, even for a first version I'd say. Shouldn't be too hard to do I suppose?

tom-anders commented 2 months ago

Yes the scale should apply to the arrow body and tip at the same time, even for a first version I'd say. Shouldn't be too hard to do I suppose?

This PR already does that - I meant that we might want to allow different scales for body and tip in the future

tom-anders commented 2 months ago

Yes the scale should apply to the arrow body and tip at the same time, even for a first version I'd say. Shouldn't be too hard to do I suppose?

This PR already does that - I meant that we might want to allow different scales for body and tip in the future

(line 79 scales the body, line 94 scales the tip)

tom-anders commented 2 months ago

Sorry, I should've included screenshots from the beginning to make things clearer, here's an arrow with scale=0.5:

smallArrow

and here's one with scale=1.5:

bigArrow

veloce commented 2 months ago

Why would we want to scale differently the body and the tip? No it's fine like that.

In your screenshot I see a thin white separation between the rectangle and the triangle. I don't think the current implementation has this issue. In any case, could you check that? Thanks.

tom-anders commented 2 months ago

Why would we want to scale differently the body and the tip? No it's fine like that.

In your screenshot I see a thin white separation between the rectangle and the triangle. I don't think the current implementation has this issue. In any case, could you check that? Thanks.

Yeah you're right, it at first looked to me like the old lichess app uses different scalings there, but at a closer look that was just my brain playing tricks.

Will take a look at the separation issue!

tom-anders commented 2 months ago

In your screenshot I see a thin white separation between the rectangle and the triangle. I don't think the current implementation has this issue. In any case, could you check that? Thanks.

Ok I tested this again, and the main branch has the same issue (see below). This is probably because I took the screenshot from a Linux desktop build, not my phone.

Screenshot_2024-07-04_12-48-53

veloce commented 2 months ago

If the problem is only on linux, that's fine. Can you please send a screenshot from an emulator or device with the scaling to show it's fine? Thanks.

tom-anders commented 2 months ago

Heres a screenshot from https://github.com/lichess-org/mobile/pull/828, doesn't look like it has the issue. Let me know if you want a screenshot of the example app running on Android instead.

veloce commented 2 months ago

No that's fine, thanks.

tom-anders commented 2 months ago

All done, thanks for the review!