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

refactor paths #76

Merged
merged 3 commits into from
Aug 2, 2023
Merged

refactor paths #76

merged 3 commits into from
Aug 2, 2023

Conversation

AI-WAIFU
Copy link

@AI-WAIFU AI-WAIFU commented Jul 27, 2023

Add compact, short information about your PR for easier understanding:

-This PR refactors how paths are instantiated and discovered both for the minetest executable and the minetester python package

To do

Ready for Review

  • Add --cache flag to specify location of the media cache directory
  • Add support for --cache flag in minetester
  • Move most minetester runtime artefacts to a default artefacts directory
  • Refactor minetest_env class

How to test

run readme up to make demo, both with a local install and wheel install, verify that no directories are created outside of the artefacts directory and that the wheel binary is being used.

@AI-WAIFU AI-WAIFU requested review from rk1a and JosiahWI July 27, 2023 15:32
Copy link

@rk1a rk1a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested running the demo from local repo and from installed package. LGTM

shape=(self.display_size[1], self.display_size[0], 3),
dtype=np.uint8,
)
self._configure_obs_space()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternative name suggestion: _configure_spaces()

if self.config_path is None:
self.config_path = os.path.join(self.root_dir, f"{self.unique_env_id}.conf")
self._set_artefact_dirs(artefact_dir, world_dir, config_path, config_dict) #Stores minetest artefacts and outputs
self._set_minetest_folders(minetest_root) #Stores actual minetest dirs and executable
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternative name suggestion: _set_minetest_dirs

)

def _set_artefact_dirs(self, artefact_dir, world_dir, config_path, config_dict):
if artefact_dir is None:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be shortened self.artefact_dir = artefact_dir or other_path

@AI-WAIFU AI-WAIFU merged commit b0254fd into develop Aug 2, 2023
1 check passed
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.

None yet

2 participants