triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.36k stars 399 forks source link

Consolidate included images #5048

Closed ron-murhammer closed 5 years ago

ron-murhammer commented 5 years ago

Consolidate images as we have them in 2 different places: https://github.com/triplea-game/triplea/tree/master/game-headed/assets https://github.com/triplea-game/assets

DanVanAtta commented 5 years ago

They need to be downloaded, run ./gradlew downloadImages

ron-murhammer commented 5 years ago

@DanVanAtta Why do they need to be downloaded? All the other asset images are included in the assets folder directly?

https://github.com/triplea-game/triplea/tree/master/game-headed/assets

DanVanAtta commented 5 years ago
ron-murhammer commented 5 years ago

@DanVanAtta I'm not necessarily against the direction here but have 2 issues:

  1. Inconsistency where most of the existing binary files are in the included assets folder
  2. Not clear that some extra gradle command needs run to pull in necessary images when running from source
DanVanAtta commented 5 years ago

The existing files should be migrated. It's not appropriate to migrate them all ad-hoc as soon as the first latest example comes along. It's also not right to never download them because they are all checked in and to forever continue that. It's a low ROI activity to migrate them all just for consistency. Rock and a hard place.

FWIW ./gradlew run will execute the download images command.

The change could have been better advertised.

ron-murhammer commented 5 years ago

I guess I prefer consistency as a few small images in this repo or in a separate asset repo doesn't matter much. Otherwise it can be confusing to other developers why images are in a bunch of different places. I guess I'll update this issue to instead say consolidate images.

DanVanAtta commented 5 years ago

As part of the consolidation, we also have images in src/main/resources, eg: ./src/main/resources/org/triplea/game/client/ui/javafx/images/join_game.png

DanVanAtta commented 5 years ago

Just to be sure, https://github.com/triplea-game/assets is the desired consolidation location. We can recreate that repo pretty readily and avoid any space limitation, we also already download assets from that location as part of the full build.

DanVanAtta commented 5 years ago

Was looking for a way to get eclipse to run the downloadImages task automatically on build. It looked like there might be a gradle config to do this, I was not able to get eclipse working well enough to test this out.

modified: build.gradle
──────────────────────────────────────────────────────────────────────────────────────────
@ build.gradle:9 @ plugins {
    id 'io.franzbecker.gradle-lombok' version '2.0' apply false
    id 'net.ltgt.errorprone' version '0.8.1' apply false
    id 'com.diffplug.gradle.spotless' version '3.23.0' apply false
+    id 'eclipse'
}

apply from: 'gradle/scripts/yaml.gradle'
dan@dan-desk:~/work/triplea$ gd 3
──────────────────────────────────────────────────────────────────────────────────────────
modified: game-headed/build.gradle
──────────────────────────────────────────────────────────────────────────────────────────
@ build.gradle:17 @ ext {
    releasesDir = file("$buildDir/releases")
}

+eclipse {
+   autoBuildTasks downloadImages
+}
DanVanAtta commented 5 years ago

All images and mp3 files from: https://github.com/triplea-game/assets Are now in: https://github.com/triplea-game/triplea/tree/master/game-headed/assets

We can close this issue or keep tracking until all images are in the assets repo.

DanVanAtta commented 5 years ago

" until all images are in the assets repo."

To clarify, not all images in game-headed are located in the assets folder.

DanVanAtta commented 5 years ago

The two locations noted in the description are consolidated! Closing, further consolidation of all the other locations is an outstanding technical debt.