Response to reviewers

.github/workflows/R-CMD-check.yaml

@@ -2,9 +2,13 @@
 # Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
 on:
  push:
- branches: [main, master]
+ branches:
+ - '*'
+ - '!gh-pages'
  pull_request:
- branches: [main, master]
+ branches:
+ - '*'
+ - '!gh-pages'
 
 name: R-CMD-check
 

.github/workflows/run-examples.yaml

@@ -0,0 +1,78 @@
+on:
+ push:
+ branches:
+ - '*'
+ - '!gh-pages'
+ pull_request:
+ branches:
+ - '*'
+ - '!gh-pages'
+
+name: run-examples
+
+jobs:
+ run-examples:
+ runs-on: ${{ matrix.config.os }}
+
+ name: ${{ matrix.config.os }} (${{ matrix.config.r }})
+
+ strategy:
+ fail-fast: false
+ matrix:
+ config:
+ - {os: ubuntu-20.04, r: 'release', rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest"}
+
+ env:
+ R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
+ RSPM: ${{ matrix.config.rspm }}
+
+ steps:
+ - uses: actions/checkout@v2
+
+ - uses: r-lib/actions/setup-r@v1
+ with:
+ r-version: ${{ matrix.config.r }}
+
+ - uses: r-lib/actions/setup-pandoc@v1
+
+ - name: Query dependencies
+ run: |
+ install.packages('remotes')
+ saveRDS(remotes::dev_package_deps(dependencies = TRUE), ".github/depends.Rds", version = 2)
+ writeLines(sprintf("R-%i.%i", getRversion()$major, getRversion()$minor), ".github/R-version")
+ shell: Rscript {0}
+
+ - name: Cache R packages
+ if: runner.os != 'Windows'
+ uses: actions/cache@v2
+ with:
+ path: ${{ env.R_LIBS_USER }}
+ key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }}
+ restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-
+
+ - name: Install system dependencies
+ if: runner.os == 'Linux'
+ run: |
+ while read -r cmd
+ do
+ eval sudo $cmd
+ done < <(Rscript -e 'writeLines(remotes::system_requirements("ubuntu", "20.04"))')
+ - name: Install dependencies
+ run: |
+ remotes::install_cran("devtools")
+ remotes::install_deps(dependencies = TRUE)
+ remotes::install_cran("rcmdcheck")
+ shell: Rscript {0}
+
+ - name: Check
+ env:
+ _R_CHECK_CRAN_INCOMING_REMOTE_: false
+ run: devtools::run_examples(run_dontrun = TRUE, run_donttest = TRUE)
+ shell: Rscript {0}
+
+ - name: Upload check results
+ if: failure()
+ uses: actions/upload-artifact@main
+ with:
+ name: ${{ runner.os }}-r${{ matrix.config.r }}-results
+ path: examples

DESCRIPTION

@@ -1,10 +1,21 @@
 Type: Package
 Package: unifir
-Title: A Unifying API for Calling 'Unity' from R
-Version: 0.1.0
-Authors@R: 
- person("Michael", "Mahoney", , "mike.mahoney.218@gmail.com", role = c("aut", "cre"),
- comment = c(ORCID = "0000-0003-2402-304X"))
+Title: A Unifying API for Calling the 'Unity' '3D' Video Game Engine
+Version: 0.2.0
+Authors@R: c(
+ person(given = "Michael", 
+ family = "Mahoney", 
+ email = "mike.mahoney.218@gmail.com", 
+ role = c("aut", "cre"),
+ comment = c(ORCID = "0000-0003-2402-304X")),
+ person(given = "Will",
+ family = "Jones",
+ role = c("rev"),
+ comment = c("Will reviewed the package (v. 0.2.0) for rOpenSci, see <https://github.com/ropensci/software-review/issues/521>")),
+ person(given = "Tan",
+ family = "Tran",
+ role = c("rev"),
+ comment = c("Tan reviewed the package (v. 0.2.0) for rOpenSci, see <https://github.com/ropensci/software-review/issues/521>")))
 Description: Functions for the creation and manipulation of scenes and objects
  within the 'Unity' '3D' video game engine (<https://unity.com/>). Specific
  focuses include the creation and import of terrain data and 'GameObjects' as

NEWS.md

@@ -1,3 +1,28 @@
+# unifir 0.2.0
+
+* Improvements and bug fixes:
+ * `find_unity()` now doesn't escape its Unity path (so the string returned 
+ is the actual path to the Unity engine, not a quoted version). 
+ Accordingly, `action()` now wraps `unity` in `shQuote()`. (#4)
+ * `add_default_tree()` now imports its trees standing upright by default. 
+ If you manually set `x_rotation` to 0, however, the trees will import as 
+ sideways as ever. (#7)
+ * Examples are now tested (and work) (#8 1d5b1f3)
+ * `create_terrain()` handles non-local terrain files (#6)
+ * `unifir_prop()` now checks to make sure `script` exists and is a 
+ `unifir_script`. Previously this errored with a baffling message about
+ long vectors.
+* Documentation changes:
+ * Vignettes have been fleshed out, and a full example added to the 
+ user-facing vignette. (#7)
+ * Return values are better documented (#5)
+ * Functions are consistently linked and documentation formatting is now
+ more consistent (#5)
+ * `find_unity()` doesn't now have a weird break in its documentation 
+ sections
+ * The README now links to vignettes and explains why anyone would want
+ to deal with Unity in the first place
+
 # unifir 0.1.0
 
 * Added a `NEWS.md` file to track changes to the package.

R/action.R

@@ -1,6 +1,7 @@
 #' Build and execute a `unifir_script`
 #'
-#' @param script The `unifir_script` object to build and execute.
+#' @param script The `unifir_script` object (as generated by [make_script])
+#' to build and execute.
 #' @param write Boolean: Write the generated script to a file?
 #' @param exec Boolean: Execute the script inside of the Unity project? Note
 #' that if `write = FALSE`, `exec` cannot be `TRUE`.
@@ -24,6 +25,10 @@
 #' }
 #' }
 #'
+#' @return If `exec = FALSE`, the original `unifir_script` object passed to
+#' `script`. If `exec = TRUE`, the same `unifir_script` object with its
+#' props replaced by the C# they generate.
+#'
 #' @export
 action <- function(script, write = TRUE, exec = TRUE, quit = TRUE) {
  debug <- check_debug()
@@ -107,7 +112,7 @@ action <- function(script, write = TRUE, exec = TRUE, quit = TRUE) {
  if (exec) {
  output <- system(
  paste0(
- find_unity(),
+ shQuote(find_unity()),
  " -batchmode",
  if (quit) " -quit",
  " -projectPath ",

R/add_player.R → R/add_asset.R

@@ -4,22 +4,22 @@
 #' https://github.com/mikemahoney218/unity_assets/
 #' to a Unity scene.
 #'
-#' In effect, these functions provide a thin wrapper across `instantiate_prefab`
-#' and `import_asset`. By providing the directory an asset is stored in, and the
+#' In effect, these functions provide a thin wrapper across [instantiate_prefab]
+#' and [import_asset]. By providing the directory an asset is stored in, and the
 #' path to the prefab file once that directory has been copied into Unity,
 #' these files will add prefabs to specified locations throughout the scene.
 #' This function will also download the necessary assets
 #' and handles specifying file paths.
 #'
-#' `add_default_player` adds "player" controllers to a Unity scene.
-#' `add_tree` adds tree GameObjects.
+#' [add_default_player] adds "player" controllers to a Unity scene.
+#' [add_default_tree] adds tree GameObjects.
 #'
 #' @inheritParams instantiate_prefab
 #' @inheritParams import_asset
-#' @param controller Which controller to use. `Player`, the default,
-#' is a simple first-person controller. `FootstepsPlayer` adds footsteps
-#' to this controller, while `JetpackPlayer` adds a "jetpack" with limited
-#' fuel. `Third Person` lets you control a small cylinder in third person.
+#' @param controller Which controller to use. "Player", the default,
+#' is a simple first-person controller. "FootstepsPlayer" adds footsteps
+#' to this controller, while "JetpackPlayer" adds a "jetpack" with limited
+#' fuel. ""Third Person" lets you control a small cylinder in third person.
 #' @param tree Which tree to use. There are currently 12 generic tree objects
 #' available, named "tree_1" through "tree_12". The number of a tree (1-12)
 #' can be specified instead of the full name.
@@ -34,16 +34,18 @@
 #' # First, create a script object.
 #' # CRAN doesn't have Unity installed, so pass
 #' # a waiver object to skip the Unity-lookup stage:
-#' script <- make_script("example_script",
-#' unity = waiver()
-#' )
+#' script <- make_script("example_script", unity = waiver())
 #'
 #' # Now add props:
 #' script <- add_default_player(script)
 #' script <- add_default_tree(script, 1)
 #' script <- save_scene(script)
 #'
 #' # Lastly, execute the script via the `action` function
+#'
+#' @return The `unifir_script` object passed to `script`, with props for adding
+#' assets appended.
+#'
 #' @rdname add_asset
 #' @export
 add_default_player <- function(script,
@@ -126,6 +128,7 @@ add_default_tree <- function(script,
  tree,
  paste0(tree, ".prefab")
  )
+ if (missing(x_rotation)) x_rotation <- -90
 
  stopifnot(all(unique(tree) %in% available_assets))
 

R/add_light.R

@@ -15,14 +15,16 @@
 #' # First, create a script object.
 #' # CRAN doesn't have Unity installed, so pass
 #' # a waiver object to skip the Unity-lookup stage:
-#' script <- make_script("example_script",
-#' unity = waiver()
-#' )
+#' script <- make_script("example_script", unity = waiver())
 #'
 #' # Now add props:
 #' script <- add_light(script)
 #'
 #' # Lastly, execute the script via the `action` function
+#'
+#' @return The `unifir_script` object passed to `script`, with props for adding
+#' lights appended.
+#'
 #' @export
 add_light <- function(script,
  light_type = c(

R/add_texture.R

@@ -3,7 +3,7 @@
 #' This function adds a helper method, `AddTexture`, to the
 #' C# script. This function is typically used to add textures to
 #' heightmaps in a Unity scene, for instance by functions
-#' like `create_terrain`. It requires some arguments be provided
+#' like [create_terrain]. It requires some arguments be provided
 #' at the C# level, and so is almost always called with `exec = FALSE`.
 #'
 #' @inheritParams new_scene
@@ -22,6 +22,10 @@
 #' script <- add_texture(script)
 #'
 #' # Lastly, execute the script via the `action` function
+#'
+#' @return The `unifir_script` object passed to `script`, with an `AddTexture`
+#' method appended.
+#'
 #' @export
 add_texture <- function(script,
  method_name = NULL,

R/associate_coordinates.R

@@ -5,12 +5,11 @@
 #' can only import terrain as square tiles of side 2^x + 1, for x between 5 and
 #' 12. As a result, importing objects into a Unity scene so that they align
 #' with terrain surfaces is trickier than you'd expect.
-#'
 #' This function "associates" the XY coordinates from some `sf` object, likely a
 #' point data set, with some raster object.
 #'
 #' @param object The `sf` object to take coordinates from. The object will be
-#' reprojected to align with `raster`.
+#' reprojected (via [sf::st_transform]) to align with `raster`.
 #' @param raster A raster or file path to a raster to associate coordinates
 #' with. Note that different rasters will produce different coordinate outputs;
 #' you should run this function with the same raster you plan on bringing into
@@ -22,7 +21,7 @@
 #' @return A data.frame with two columns, X and Y, representing the re-aligned
 #' coordinates. If `object` is point data (or anything object that
 #' [sf::st_coordinates] returns a single row for each row of), these rows will
-#' be in the same order as `object` (and so can be appended via `cbind`).
+#' be in the same order as `object` (and so can be appended via [cbind]).
 #'
 #' @examples
 #' \dontrun{
@@ -32,7 +31,11 @@
 #' lat = runif(100, 44.04905, 44.17609),
 #' lng = runif(100, -74.01188, -73.83493)
 #' )
-#' simulated_data <- sf::st_as_sf(simulated_data, coords = c("lng", "lat"))
+#' simulated_data <- sf::st_as_sf(
+#' simulated_data,
+#' coords = c("lng", "lat"),
+#' crs = 4326
+#' )
 #' output_files <- terrainr::get_tiles(simulated_data)
 #' temptiff <- tempfile(fileext = ".tif")
 #' terrainr::merge_rasters(output_files["elevation"][[1]], temptiff)

R/create_terrain.R

@@ -87,7 +87,8 @@ create_terrain <- function(script,
  .open = "%",
  .close = "%",
  method_name = prop$method_name,
- heightmapPath = prop$parameters$heightmap_path,
+ heightmap_path = prop$parameters$heightmap_path,
+ base_path = basename(prop$parameters$heightmap_path),
  x_pos = prop$parameters$x_pos,
  z_pos = prop$parameters$z_pos,
  width = prop$parameters$width,