Student Group Studying TripleA

jtkenny

@lafayette Our assignment states that we need to open and complete two to three pull requests. The first one can be small for a small fix or documentation update. The others must be substantive, so most likely the answer to your question is a combination of both.

LaFayette

@jtkenny Sounds like the first PR is perhaps something more to go through the PR process and be something of a minimal update. That is really reasonable since there are a number of steps to get up and running and to be able to push a branch & PR it.

"substantive" could use clarification. We try to have all updates be incremental and that requires some planning. Ideally all PRs are under 500 LOC so that they can be reviewed in one session by a reviewer. This implies that a 2000 LOC change would want to be split up across at least 4 PRs. For example, if adding a new feature, the first PR might be adding the feature flag and an if statement where the update will occur, eg:

if(ClientSettings.someFeatureFlag.getValue().orElse(false)) {
   // TODO: implementation code will be filled out here
}

Then a next update would start filling out an implementation, followed by testing and then finally a last PR that removes the if block (removing the "feature flag") and any cleanup.

Some fixes in the code base while small, could be pretty 'substantive' (ie: consequential). There are lots of automatic error reports that are uploaded that could be the starting point for relatively small-medium sized updates.

Happy to give guidance if you indicate area of interest.

Cernel

I think sounds are the most neglected part of TripleA.

Here it is a feature request of mine, on the matter:
https://forums.triplea-game.org/topic/275/units-specific-placed-sounds

Other things would be getting a sound every time you click on units and having background sounds, general, per turn, per phase (keeping playing only for as long as the phase is active) and per phase and turn.

Another thing would be able to order the priority by which support bonuses/maluses are assigned (For example, if there is an artillery which can support an infantry or an elite, to decide which gets the support.). I've a feature request open for that too.

jtkenny

@lafayette I think what our professor meant by substantive would just be that is a more major change rather than just a small bugfix. I am assuming that this could be spread across multiple PRs if necessary to meet your requirements for a PR. Nevertheless, if there are any small issues we could start looking at for our first PR, just let us know and we would be happy to get started.

LaFayette

@jtkenny Understood regarding a substantive update. Let's discuss before you get started as much as possible to help grease the wheels. It can be very tricky to break up larger PRs into smaller, but we can try our best.

FWIW, some studies have shown that code review effectiveness drops off dramatically after about 30 minutes (and that reviewing for more than hour is beyond diminishing returns). Similar studies have shown that a typical person can review up to about 500 lines in 30 minutes. Thus, to have a PR reviewed in one sitting (ie: in one day) requires it be under 500 lines. Reviews split across several days is difficult for everyone involved. How small to split up PRs is a preference of the project and organization. Generally we want them about as small as possible (we have super limited review capacity. It wouldn't be bad to have other members of your team do reviews of PRs you submit, that would help for some first passes and give you valuable experience. Coding, writing tests, and code reviewing are all independent skills)

For good issues to tackle, it would be really nice if we labelled more as 'good first time issues', but the labelling/intake of issues is pretty neglected for a lot of reasons. Regardless, the 'error reports' are all valid problems, fixing any one would be a good bug fix. Look for the error reports with a '2.6' version: https://github.com/triplea-game/triplea/issues?q=is%3Aissue+is%3Aopen+label%3A"Error+Report"+2.6%2B

For the error reports, the biggest challenge is usually reproducing the error. Once you can repro, it's easier to discuss a fix and/or just fix it directly at that point.

beelee

Hi @jtkenny

Can you keep us updated with what you guys want todo and have done so people can follow along ?

Thanks

LaFayette

Checking in:
@jtkenny has everyone in your group done the following?

set up an account in github
cloned the code
set up IDE (intellij preferred)
ran the 'verify' script to run all tests
- installed docker and have started a local databsae
able to launch the game client
able to launch the lobby

The above is a bit the ABCs before being able to make any updates. With luck the developer setup documentation makes the above very easy and straight forward. If not, please ask and/or update the setup documentation to fill in any gaps

I was thinking as well about the architecture documentation task that you have as part of your assignment. It would be great to have that be included in the official docs! Though, it might be a lot more tractable to look at the architecture of a single sub-system rather than the whole app. For example, the lobby has a pretty well defined architecture, network communication has a pretty well defined one, and how map parsing is done to get game objects has a pretty well defined design as well.

ebbe

@thedog @jtkenny
1. Displaying how many of a unit can purchased and blocking the purchase of too many units see the image below, the units with a ≤
2. calculating how many units can be placed this turn, in this example 11.

These are on the top of my Wishlist too !!!! ; )

jtkenny

@beelee This is the issue we selected to try to work on, although so far we have been unable to replicate the error: https://github.com/triplea-game/triplea/issues/11097

So far we are thinking that maybe this is because we are on the 2.6 version of the game, and this may be on a later versions, but we also haven't been able to figure out how to move forward to this version.

jtkenny

@lafayette Yes our entire group have completed these steps for the most part. I have been able to run the application in IntelliJ on my computer, and I think everyone else has as well.

TheDog

@jtkenny said in Student Group Studying TripleA:

but we also haven't been able to figure out how to move forward to this version.

Not sure what you mean, but I think you need to install multiple versions of the TripleA game
So download a Triple version, but when installing it, use the version as the install path
eg install to TripleA-2.6+14163
like;

and therefore download the source of the version that you want to change.

Im not sure the one issue you have selected, is actually an issue. I'm sad and have no life and I use TripleA daily and I have not seen this error message, speaking as a xml coder of many maps and many errors.

Being selfish the biggest impact you could make for us TripleA-ers is in the 2nd post of this thread, as lots of maps have this style of unit restriction.

If you met this restriction and gone over a unit limit, TripleA resets all your purchases and you have to start over, not a good experience for a player to have.

LaFayette

@jtkenny There is a tag pushed on each release that matches the release version number. You should be able to checkout the exact version via: git fetch upstream; git checkout 2.6+14163 (I'm assuming you named the 'upstream' remote as 'upstream', git remote -v should indicate the name to use).

The version in the error report does correspond to the game-client version that sent the error report, seemingly this (issue #11097) should be something that you can reproduce in 2.6.

It might be most fruitful to see how and where returnFire could be null. That might then give you a path for reproducing the issue.

Re: local dev setup

Just want to be sure that your team is all able to run verify script and not just launch the game from IDE. The verify script is useful since it runs all tests & static checks in one-go. It has more requirements then just running the game since you need to install docker (which then provides you with a local databse). If that works, then you'll be able to launch all components of the stack and not just the client application. To add to that, verify is particularly useful for PR verification - it can be quite endless to push updates and wait for the build to fail on relatively random issues compared to running verify locally (which is all in the name of reducing developer iteration time)

LaFayette

@jtkenny for issues to work on, also please consider the problem tracker: https://github.com/orgs/triplea-game/projects/3

The 'fix sooner' column has some relatively tractable items that are all relatively high impact. The 'standard fix' items are all similarly good items to pick up.

Even if the 'problem tracker' is not as complete as it could be, I did want to point this out as another reservoir of items that could be picked up.

LaFayette

One more item, it's a bit easier than ever to find the '2.6' error reports, we added some labels: https://github.com/triplea-game/triplea/issues?q=is%3Aopen+is%3Aissue+label%3A"Error+Report"+label%3A2.6

jtkenny

@lafayette I've been trying to run the verify script, but the the database startup times out and aborts and so far I have been able to run it.

LaFayette

@jtkenny I would guess the DB is not starting.

The startup script is mostly a wrapper around this command:

  docker run \
    --rm -d \
    --name="database" \
    -e "POSTGRES_PASSWORD=postgres" \
    -p "5432:5432" \
    -v "$(pwd)/$(dirname "$0")/sql/init/:/docker-entrypoint-initdb.d/" \
    "postgres:10"

A quick debug would be to run without the folder mount at first:

  docker run \
    --rm -d \
    --name="database" \
    -e "POSTGRES_PASSWORD=postgres" \
    -p "5432:5432" \
    "postgres:10"

Let us know how that goes

Lydia McCleary

Hi all, I am also a member of the student group. I have a few questions regarding TripleA's availability. Has TripleA ever gone down? How long is TripleA's typical down time (either when changes are being committed or unintentional outages)?

beelee

Hi @lydia-mccleary

The lobby goes down sometimes. Rarely for more than a few hours. Dan probably give you a more detailed answer.

LaFayette

@lydia-mccleary

Has TripleA ever gone down?

Yes. Though, in terms of availability, perhaps to some surprise it is quite high (out of the year, perhaps only a day or two of downtime). For better or worse, we do not measure the availability of the lobby, so a concrete answer is unavailable. Though, we are pretty quick to get reports about the lobby not being available, and there are not very many over the last 4 years. These are the ones I could find:

https://github.com/triplea-game/triplea/issues/10804 < deployment configuration problem, lobby database password got out of sync with lobby application

https://github.com/triplea-game/triplea/issues/9708 < regional issue relating to DNS resolution of our domain

https://forums.triplea-game.org/topic/1239/where-is-the-lobby-down-topic < linode "maintenance" (note, the person was asking 'where is the lobby down' topic in order to be able to post about the lobby being down!)

How long is TripleA's typical down time (either when changes are being committed or unintentional outages)?

I think the events above were all hours. I'm not aware of any occasional blips in the software, generally downtimes are discrete events (rather than say the lobby crashing on a recurring basis)

either when changes are being committed

Lobby needs a restart to pick up updates. Last time we did this was almost 3 years ago (maybe more even, lobby is running v2.4). We can have users log into a new lobby now without downtime but there is no shared state between different lobby versions. Meaning, a player might be the first to rejoin into a new version of the lobby and be the only one there.

For 2.6, we are pretty close to doing a lobby cut-over. It should be zero downtime. Some of this should play out in the near future. Happy to answer further questions.

jtkenny

@lafayette Sorry about the delayed response, we have been very busy with exams and writing an essay.

I ran the script you gave and it started the database, but I was still unable to run the verify script because it seemed to stop and then try to start the database again.

On another note, I left a comment on the issue we were going to start on. Is there a particular naming convention I should use for the branch we make for the PR?