search

vasl-developers / vasl

Virtual Advanced Squad Leader
http://vasl.info/
GNU Lesser General Public License v2.1
66 stars 28 forks source link

Fixed bug when moving a concealment counter unconceals counters that shouldnt be #1851

Closed geezer09 closed 2 weeks ago

geezer09 commented 2 weeks ago

The code looked at counters below the old location of the concealment counter and did not consider if there was another ? counter above them. I made a slight change so that when a concealment counter is moved within a stack, we look at the top of the stack to see if there are any other concealment counters, if there are, then we dont change the concealment status of the counters below them.

closes #1848

zgrose commented 2 weeks ago

I have an edge case to consider in your unit tests:

Screenshot 2024-11-02 164915

zgrose commented 2 weeks ago

Perhaps a slightly better edge case since no one would move that ground level concealment counter down....

Screenshot 2024-11-02 165143

geezer09 commented 2 weeks ago

Humm good point. I did a quick look and it seems like level markers are currently being ignored. I think in the above situation, if everybody was unconcealed and you concealed the squad at 1st level everyone gets concealed.

Off the top of my head, the correct solution is to implement a prototype for pieces that create a different location within a hex and use that when checking for concealment so that ? Counters only affect units within that location.

Thoughts?

zgrose commented 2 weeks ago

Semantically, what does it mean to move a ? down a level in a stack... I get that various combinations are weird, but maybe the question is why does someone move a counter down vs move a counter up. I don't have a great answer because the cardboard and VASSAL don't align, but sometimes problems don't need to be solved, just explained.

(edit) that is to say, a masked counter should probably always be a masked counter until the player says "you aren't masked anymore", but that's just my personal take

geezer09 commented 2 weeks ago

Hum I think I understand what you're trying to get at, but honestly it seems almost harder to try and figure the semantics of why certain actions are taken then to make sure the rules are applied every time one is taken.

You might be able to take some shortcuts in the first case, but it’s harder to make interpretation errors in the second.

In any case, it seems like different locations within a hex should be treated separately regardless of which approach you decide to take.

Get Outlook for iOShttps://aka.ms/o0ukef

From: Zoltan Grose @.> Sent: Saturday, November 2, 2024 4:16:55 PM To: vasl-developers/vasl @.> Cc: Joseph Larochelle @.>; State change @.> Subject: Re: [vasl-developers/vasl] Fixed bug when moving a concealment counter unconceals counters that shouldnt be (PR #1851)

Semantically, what does it mean to move a ? down a level in a stack... I get that various combinations are weird, but maybe the question is why does someone move a counter down vs move a counter up. I don't have a great answer because the cardboard and VASSAL don't align, but sometimes problems don't need to be solved, just explained.

— Reply to this email directly, view it on GitHubhttps://github.com/vasl-developers/vasl/pull/1851#issuecomment-2453221898, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A5CSCRIDUQYTTQ2GT4ZXU7DZ6VMONAVCNFSM6AAAAABRCAAKJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJTGIZDCOBZHA. You are receiving this because you modified the open/close state.Message ID: @.***>

derimmer commented 1 week ago

Humm good point. I did a quick look and it seems like level markers are currently being ignored. I think in the above situation, if everybody was unconcealed and you concealed the squad at 1st level everyone gets concealed.

Yes, this is what happens at present; level markers are ignored.

Off the top of my head, the correct solution is to implement a prototype for pieces that create a different location within a hex and use that when checking for concealment so that ? Counters only affect units within that location.

Most of the other locations are already created within the los engine code and so are available for use by the Concealment code IFF the board is los enabled. So, this is probably not a useful approach as many boards are not los ready.

Thoughts?

Some of the work I did in trying to implement No ROI might be of use here. Or more likely, can be improved after you figure out how to do this.

geezer09 commented 1 week ago

I suggest you pull from dev670 and play with concealment counters and level counters. You’ll see they act a bit differently than before. I tried to simulate more closely in person play. If you place your counter somewhere it would not be concealed (ie not below a concealment counter), it will automatically get unconcealed. Level counters are taken into account.

There used to be a bit of inconsistent behaviour with counters remaining concealed until manually unconcealed. I think there are some pros and cons to both approaches, but all things being equal something that is closer to real life gaming has an advantage in my opinion.

derimmer commented 4 days ago

I have no pulled all of your 670 changes into my dev environment and have a beta build running. I will start to work through your changes and get them a quick test.