Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Development enviroment improvements #843

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

Wartori54
Copy link
Contributor

This PR's goal is to improve the Everest development experience, it does the following:

  • Introduces a new mode for MiniInstaller where you can select what parts of the installation process to execute, greatly reducing installation times.
  • Splits the MiniInstaller project into multiple files, instead of concentrating it all in a single 1k+ line file.
  • Introduces two new build scripts for Linux in order to easily automate the installation and development of Everest.

Ideally build scripts for other environments would also be added later on. I believe that the scripts can be executed with some trickery under OS X and windows (using WSL or similar) though.

Feedback on what features to add/remove from the build scripts is greatly appreciated.

@ErynGalen
Copy link

I'de rather see the scripts be something like this. (file attached)
Maybe CELESTEGAMEPATH shouldn't have a default value.

config.sh:

# Feel free to adjust these

EVERESTPATH=${EVERESTPATH:-"$(dirname "$0")/../.."}

CELESTEGAMEPATH=${CELESTEGAMEPATH:-"$EVERESTPATH/_celestegame"}

# For TAS consistency it usually is necessary to build in Release mode, but for
# anything else Debug will suffice and provide better debugger support
CONFIGURATION=${CONFIGURATION:-"Debug"}

OUTDIR=${OUTDIR:-"$EVERESTPATH/artifacts/Everest"}

complete_build_and_install.sh (there was a typo in the file name):

#!/usr/bin/env bash

set -e

# This script is used to completely install or upgrade an celeste installation
# from the source files; you must call this script first before you're able to
# use quick_patch.sh

source config.sh

# It is sufficient to build only these two projects currently, all the others
# will be built and copied as part of the build process of these two.
TARGET_PROJECTS_PATH=(
  "$EVERESTPATH/Celeste.Mod.mm/Celeste.Mod.mm.csproj"
  "$EVERESTPATH/MiniInstaller/MiniInstaller.csproj"
)

ARTIFACT_DIR="$OUTDIR"

echo "Building with dotnet"
for PROJECT in "${TARGET_PROJECTS_PATH[@]}" ; do
    echo "Building $PROJECT"
    dotnet publish "$PROJECT" -o "$ARTIFACT_DIR" -c $CONFIGURATION
done

echo "Copying files"
cp -r "$ARTIFACT_DIR"/* "$CELESTEGAMEPATH"/

if [ "$2" != "--skipinstall" ]; then
    echo "Installing everest"
    cd "$CELESTEGAMEPATH"
    echo "Calling MiniInstaller-linux"
    ./MiniInstaller-linux
fi 

quick_patch.sh:

#!/usr/bin/env bash

set -e

# This script is used to quickly build and setup all the necessary files for any
# changes in Celeste.Mod.mm to be applied.
#
# Note: there are some edge cases, such as modifying patches for FNA, that are
# not covered by this script by default. This specific edge case can be fixed by
# adding "fna" to the `MiniInstaller-linux` call, refer to fast-mode
# documentation for more information.

source config.sh

ARTIFACT_DIR="$OUTDIR/quick_patch"

TARGET_PROJECT_PATH="$EVERESTPATH/Celeste.Mod.mm/Celeste.Mod.mm.csproj"

# For TAS consistency it usually is necessary to build in Release mode, but for
# anything else Debug will suffice and provide better debugger support
CONFIGURATION="Debug"

if [ "$1" == "--first-run" ] || [ ! -f $ARTIFACT_DIR/MonoMod.Backports.dll ]; then
  echo "Doing a complete build given that dependencies are missing"
  dotnet build "$TARGET_PROJECT_PATH" --output "$ARTIFACT_DIR" -c $CONFIGURATION
fi

echo "Building $TARGET_PROJECT_PATH without restore nor dependencies"
dotnet build "$TARGET_PROJECT_PATH" --no-restore --no-dependencies --output "$ARTIFACT_DIR" -c $CONFIGURATION

echo "Copying artifacts into game directory"
cp -r "$ARTIFACT_DIR"/* "$CELESTEGAMEPATH"/

cd "$CELESTEGAMEPATH"

if [ ! -f "./MiniInstaller-linux" ]; then
  echo "Could not find MiniInstaller-linux! Run complete_build_and_install.sh in order to set it up."
  exit 1
fi

echo "Calling MiniInstaller-linux"
./MiniInstaller-linux --fastmode maingame

@Wartori54
Copy link
Contributor Author

I intentionally tried to keep it as simple as possible, though I do agree that this would be a nice improvement, but if we make this too elaborate then porting it to other platforms wont be as easy, which is what I wanted to avoid (given that almost all code in this repo is cross-platform and this would break that pattern). Again though, I'm open to abstracting settings to a config file and maybe un-hardcoding MiniInstaller-linux so that maybe it can be run under OS X as well, if other people agree.

About the celeste path, my take is that it is more convenient for it to be assigned to an arbitrary value so that people who just want to set it up can just drop the game at that same path and don't have to modify the config (this should get documented though, will do if necessary). But at the same time hardcoding paths to arbitrary values feels "wrong" as well.

(And thank you for pointing the typo, my keyboard trolled me again).

@ErynGalen
Copy link

The change I care the most about is using set -e instead of putting || exit after each command. The rest I found to be a little cleaner, that's it.

I understand the cross-platform concern, but it's not really a reason to not use UNIX shell features since the script is already not very portable.

A more portable solution would be to use Python.

@RedFlames
Copy link
Contributor

Very much in favor of what the proposed config.sh does to only give the variables their default values if they aren't set already, so that e.g. I could export those in my .bashrc or something and they'll be used from my env. Telling me to manually edit those defaults in a shell script is a bit too silly. :3

@Wartori54
Copy link
Contributor Author

Adjusted the relevant files according to the given feedback. Still unsure on what to default CELESTEGAMEPATH to, should I just leave it empty by default and panic if you don't override it?

And in the topic of using a cross-platform shell script, tools like this exist, would this be worth looking into? Would be cross platform and remove the need of adding a new language and it seems like you don't need to explicitly install it if you use it as a library on a standard project, but haven't dug too deep into this, thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants