Bug that reveals secret information to the player.

[jvoigtlaender]

I had sunk the ship at F4/G4. I never fired a F4 again. Later the AI moved its ship from F1/F2 to F3/F4. At that point, F4 suddenly turns from water into hit ship. Of course, the F3/F4 ship is not hit at all, and the AI will happily move it further in the next turns. But the rendering tells me that an AI ship is crossing the area of the sunk ship.

I wonder whether the same phenomenon happens on the player's grid (where it wouldn't reveal secret information, but simply be a wrong rendering).

Somebody please check the rendering code very carefully for this and similar possible bugs.

[jvoigtlaender]

Having played around a bit again now, observed many further rendering-related misbehaviors, such as:

• The opposite of what happened above. Namely, a ship was sunk. Another ship moves over it. I hit it at one of the overlapped cells. But that cell remains displayed as water (i.e., as part of the sunk ship, not as part of a now hit and thus immovable ship at that position).
• A ship was sunk. Another ship later moves over it. The second ship is now also sunk. All the cells of the second ship should now be deep blue. Instead, the cells in the overlap remain a transparent blue as if we didn't know for sure that they are water right now.
• (Noticed the following in replay mode, but may also be an issue in the playing mode on the player's grid:) A ship was sunk. Another ship moves over it. But the border which is supposed to indicate the area of the sunk ship is drawn over the yellow squares of the movable ship, whereas more appropriate would be the opposite.

[fanzier]

The problem here is that when ships are on top of each other, shipAt may return the wrong ship for that position. We need to consider all ships that contain the current position. But then, it is very hard to find out when they were sunk ... The cleanest solution I can think of is changing the Ship data type to the following:

data Ship =
-- ...
, shipDamage :: Array Int Time -- ^ when was damage inflicted to a position
}

newtype Time = Time (Maybe Int) -- ^ Nothing, if never hit, the corresponding turnNumber otherwise

Finding out when a ship was sunk becomes trivial. However, I'm reluctant to change a core data type at this stage of the project... So, what do you think?

[fatho]

Ships may only overlap when all but one have already been sunk, and determining if a ship has been sunk is also trivial. Why do we need to store the time?

[fanzier]

If a ship is sunk, is easy to determine. When a ship was sunk, can't be determined (without a replay) in some cases. (E.g. two sunk 2-ships on top of each other, we don't know which one was sunk first) On the other hand, for the rendering, we only need to know when the last time was that any ship through the current position was sunk. But even that can be very hard in certan circumstances. Consider the following situation: There's a 2-ship at A1 and A2, a 3-ship at A1, A2 and A3. Both of them are sunk and the sinking shot was at A1 for both of them. Then for the right opacity at A3, we need to know which ship was sunk first. And without a replay this is terribly complicated. (At least I don't know how to do it without a replay.) Or am I missing something?

[jvoigtlaender]

I'll refrain from an opinion on how to go about this (replaying or extending the datatype or whatever). Playing the customer this once. :-)

Just two general comments, in case you decide to go with an extended datatype:

• Such a refactoring may not be as painful as you assume. There is a good chance that the compiler will help you a lot in identifying all places in the code that you have to change.
• One thing to consider would be how much this inflates the size of the information that needs to be stored in the limited space available in encoded URLs.

Also, one other remark, relative to the scenario with A1, A2 and A3 in the previous comment above: Keep in mind that, in that scenario, what transparency to apply to A3 depends not only on the sinking times but also on whether or not the novice mode is turned on. Actually, this is even more acute for position B3. For it, what transparency to apply in novice mode depends not only on the sinking times of cells in the neighbourhood, but also on how the ships to which these cells belonged when they were sunk were oriented (I think).

[jvoigtlaender]

Well, one additional comment after all, opinion or not:

An option independent of managing sinking times might be to add another piece of information to each grid cell, which holds data about "when did the user last learn something about this cell, and what was it that was learned at that time". Of course, this data would have to be updated by events like hitting and sinking ships, and the specific update would depend both on whether a ship was just hit or was actually sunk, as well as on whether the novice mode was turned on or off. That option was already discussed in a meeting at some point, at which time the prevailing opinion was that you don't want to cache/maintain this information, instead want to reconstruct it from the game's history on demand. Maybe that decision is to be reconsidered now?

[fanzier]

Such a refactoring may not be as painful as you assume. There is a good chance that the compiler will help you a lot in identifying all places in the code that you have to change.

Probably, yes.

One thing to consider would be how much this inflates the size of the information that needs to be stored in the limited space available in encoded URLs.

I think it could be done without an increase in the size: An Array of Bool needs one byte per index, right? But you could also store a t :: Time in one byte, e.g. Nothing becomes 0xFF and Just i is i stored in one byte.

Overall, I think extending the Ship type is the best option at the moment. Replaying would be very complex and probably slow... But I'll wait for some more feedback before implementing anything.

[Nifdee]

I agree that extending the Ship type looks like the best option. This would allow a relatively easy computation of what should be displayed while it is easily maintainable information. It might also take up less space than adding extra information to the grid cells...

@fatho Ships may overlap as soon as one ship is sunk; this just does not happen too frequently because our AI defers sinking ships for as long as possible.

[fanzier]

As nobody objected and there's only so much time left, I'll implement the extension of the Ship type.

[fanzier]

The refactoring was indeed no problem at all. Static typing is awesome. :-)