Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Musketeer variant implementation #371

Closed
alexobviously opened this issue Nov 16, 2020 · 12 comments
Closed

Musketeer variant implementation #371

alexobviously opened this issue Nov 16, 2020 · 12 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@alexobviously
Copy link

Hi Tamas, I emailed you a few times a while ago about getting set up to add a new variant, and I've been working on it since then. I believe you're already familiar with Zied and his Musketeer variant and discussed adding it some time ago with Fabian but determined it was a lot of work. So Zied asked me to do it, and I've been mostly working on the Fairy-Stockfish implementation, and I'm pretty much done with that so I'm now getting the relevant UI bits set up etc.

So I thought I'd create this issue so I can ask you some questions and iron out some problems. I'm not really totally familiar with JS/TS so it's slow progress finding the source of problems.

Anyway, to start: I could use some help getting the right piece 'roles'/icons displaying for new pieces I'm trying to add. So I have:

  • static/piece/musketeer/musketeer0.css + empty.css
  • static/images/pieces/musketeer/*.svg for every piece
  • empty.css linked in the template and PIECE_CSS_START and cssLinkIndex set up (this was tripping me up for a while)
  • in chess.ts:
musketeer: new Variant({
        name: "musketeer", displayName: "musketeer", tooltip: _("Many new pieces with fixed gating"),
        startFen: "us******/rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR/US****** w KQkq - 0 1",
        board: "standard8x8", piece: "musketeer",
        pieceRoles: ["king", "queen", "rook", "bishop", "knight", "pawn", "leopard", "hawk", "chancellor", "archbishop", "elephant", "unicorn", "cannon", "dragon", "fortress", "spider"],
        enPassant: true, autoQueenable: true,
        icon: "`",
    }),

So my problem now is how do I associate the symbol with the piece role (e.g. 'U' -> 'unicorn')? I found roleToSan and sanToRole in chess.ts but they didn't seem to change anything. The 'S' always results in an attempt to display a piece.silver.white which of course doesn't exist in musketeer0.css and results in no piece being displayed.

I've sort of come to the conclusion that I might have to modify chessgroundx too because it looks like roles are hardcoded in there, but I'm really hoping to avoid that since I already have two packages to adapt (you're possibly wondering how I'm planning to implement the prelude phase without editing chessgroundx, but I did a test implementation a while ago and I did find a way). So I'm hoping I'm just missing some obvious thing you can help me with here.

I guess what I could do is not use any new roles, instead just pick enough random ones to represent all of the pieces and rename the pieces in musketeer0.css, but that feels messy and besides, the SAN symbols for the pieces would be wrong then. The 'roles' actually don't specify anything (e.g. behaviour) outside of linking to a piece image and SAN symbol, right?

Anyway thanks in advance. I'm looking forward to finally getting this finished!
Alex

cc. @musketeerchess if you want to follow too

@gbtami
Copy link
Owner

gbtami commented Nov 16, 2020

Telling the truth as I added more and more variants to pychess, FEN piece letters and chessgroudx roles became more and more overloaded. This led to strange situations like Shako cannon is called Chancellor in https://github.com/gbtami/pychess-variants/blob/master/static/piece/shako/shako0.css or Makruk Bishop is called Silver in https://github.com/gbtami/pychess-variants/blob/master/static/piece/makruk/makruk.css

Now i think i should just rename almost all roles to just "a-piece", "b-piece", "c-piece", etc.in chessgroundx and in all .css
(There are exceptions of course. While chessground git readme states "No chess logic inside", it is not true. So basic chess piece roles should be preserved if you don't want to touch chessground internals.)

All in all you are right, new role names (meaning: non standard 8x8 chess piece roles) don't specify anything!
But.
Mapping of letters to roles is defined here https://github.com/gbtami/chessgroundx/blob/master/src/fen.ts
There is no role for "u" and for "d" pieces. At least these 2 new should be added to types.ts

One more thing you can't avoid to code in chessgroundx is premove support of new pieces! https://github.com/gbtami/chessgroundx/blob/master/src/premove.ts
But this is a trivial task compared to modifying Fairy I think :)

@alexobviously
Copy link
Author

alexobviously commented Nov 17, 2020

Hey, thanks for the quick reply

Telling the truth as I added more and more variants to pychess, FEN piece letters and chessgroudx roles became more and more overloaded. This led to strange situations like Shako cannon is called Chancellor in https://github.com/gbtami/pychess-variants/blob/master/static/piece/shako/shako0.css or Makruk Bishop is called Silver in https://github.com/gbtami/pychess-variants/blob/master/static/piece/makruk/makruk.css

Ah I guess this makes sense, considering that the original chessground was not designed with fairy pieces in mind.

Now i think i should just rename almost all roles to just "a-piece", "b-piece", "c-piece", etc.in chessgroundx and in all .css

I agree and I think that definitely makes more sense (took me a long time to realise that these things were even hardcoded somewhere), but maybe there's just a better way to do this in general that makes more sense for chessgroundx since it deals primarily with variants that have a lot of different pieces? Maybe like a config-based approach where you pass all the roles/letters you want to chessground when you initialise it, or just remove the cg.Role thing entirely and just use letters for everything? Like I said, I'm not really a JS guy and I haven't looked at chessground in that much depth so I'm not sure how feasible that is - just an idea.

(There are exceptions of course. While chessground git readme states "No chess logic inside", it is not true. So basic chess piece roles should be preserved if you don't want to touch chessground internals.)

Okay this is the caveat I was looking for, haha. So 'basic chess piece roles should be preserved' means if I have a piece that has different movement to any preexisting pieces, I can't just assign it to 'fers' or 'silver' or something and expect it to work, right? Like you said you called the makruk bishop 'silver', but shogi silver and makruk bishop move in the same way (right?), but musketeer's spider, for example, doesn't move in the same way as any piece we have already.

Before you said this, what I was thinking was I'd make a quick temporary version by assigning all the musketeer pieces to random roles/letters so I could give Zied something to test with while we figure out a better solution, but now I'm thinking that won't work. (I guess I could just try it, but I'd have to change some bits in Fairy which I already have working and I don't want to risk it blowing up in my face if I know it's already not going to work :) )

All in all you are right, new role names (meaning: non standard 8x8 chess piece roles) don't specify anything!
But.
Mapping of letters to roles is defined here https://github.com/gbtami/chessgroundx/blob/master/src/fen.ts
There is no role for "u" and for "d" pieces. At least these 2 new should be added to types.ts

One more thing you can't avoid to code in chessgroundx is premove support of new pieces! https://github.com/gbtami/chessgroundx/blob/master/src/premove.ts
But this is a trivial task compared to modifying Fairy I think :)

Okay this is quite reassuring - if it's only the relevant parts in types.ts, fen.ts and premove.ts then I could get that done tonight. I mostly just wanted to avoid the hassle of getting three forked repos working together but the code itself is probably trivial like you say.

@alexobviously
Copy link
Author

To be clear - is it impossible for there to be two pieces/roles with the same letter?
For example, there is a piece called 'elephant' which has the letter 'e', and there is also a piece called 'elephant' in musketeer but it doesn't have the same moves, so this has to have a different letter with the current system, right?
Just making sure - in the meantime I don't think this is a huge problem but later on it might be better if there's a way around this.

@gbtami
Copy link
Owner

gbtami commented Nov 17, 2020

It is possible to use the same letter in different variants. Shako Cannon and Grand Chancellor both use letter "c" in FEN.
Chessgroundx has one to one mapping from letters to roles inside geometry groups at this moment. See here https://github.com/gbtami/chessgroundx/blob/master/src/fen.ts#L42

Btw. is Musketeer 8x8 or 8x10 variant in Fairy?

@alexobviously
Copy link
Author

Ah right, so the shako cannon can do that because it's part of a variant with different geometry. Is there an example of two pieces with the same letter that have the same geometry type? I can just do this for the time being: export function read(fen: cg.FEN, geom: cg.Geometry, musketeer = false) ... if(musketeer) roles = rolesMusketeer;

But maybe there's a cleaner way you'd prefer in the long term?

Musketeer is 8x8 in fairy. The gates aren't treated as part of the board but they look like it in the FEN, so I have to crop them out when passing from pychess to chessgroundx. I wanted to make it 8x10 initially but it was just too complicated and it broke everything.

@gbtami
Copy link
Owner

gbtami commented Nov 17, 2020

No. Shako and Grand are both 10x10 games. But because "c" is mapped to role "chancellor" in fen.ts I have to use this in https://github.com/gbtami/pychess-variants/blob/master/static/piece/shako/shako0.css

I try to keep modification in chessgroundx as little as possible compared to upstream chessground to easy later merges.
I think no new parameter needed in fen.ts

@alexobviously
Copy link
Author

Oh right sorry, I think I understand it now, hold on half an hour or so and I'll have a proposed update for chessgroundx!

@alexobviously
Copy link
Author

Does this look about right? alexobviously/chessgroundx@86ccaf8

@gbtami
Copy link
Owner

gbtami commented Nov 17, 2020

Yes, I think so.

@gbtami
Copy link
Owner

gbtami commented Nov 17, 2020

Uploaded new version 7.6.42 https://www.npmjs.com/package/chessgroundx

@alexobviously
Copy link
Author

Thanks, that's great. It's nice to have one part finalised at last! I didn't have much time today but I managed to test the integration and everything works as expected - the pieces are displaying properly now and their movement isn't affected as far as I can see.

I'm hoping to finish off all the gating tomorrow and deploy to a test server. I'll show you when I've finished but basically I am handling the musketeer gating as a separate case ('commit gates' - named by Fabian), so I'm not using the existing pockets for this. Instead I have two long pockets at the top and bottom of the board respectively and we just only render them if the variant is commit gates type.

The prelude phase is another matter entirely and I think I'm going to leave it out of the test version for now - I have something but it doesn't work reliably.

@gbtami gbtami added the enhancement New feature or request label Mar 18, 2021
@gbtami
Copy link
Owner

gbtami commented Aug 26, 2021

Musketeer support issue was closed in Fairy-Stockfish, so I close this as well.
If it will be ever reopened, we can reopen this again.

@gbtami gbtami closed this as completed Aug 26, 2021
@gbtami gbtami added the wontfix This will not be worked on label Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants