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

Picotazo patch 2 Massive update to xml file #17

Closed
wants to merge 6 commits into from

Conversation

picotazo
Copy link

@picotazo picotazo commented Mar 6, 2020

No description provided.

fixed the goof I did with some of the entities mainly the villagerv2,wandering trader and ravager.
updated the Hostile mobs section of mcpe_viz.html.template with separate section for illager type mobs  so now there are 4 main section for standard/illager/nether and the end
@paulgrahek
Copy link
Member

paulgrahek commented Mar 10, 2020

The files changes you uploaded were committed to the root of the repository, not to the location where the files were at before. See below where the files should be:
mcpe_viz.cfg -> data/mcpe_viz.cfg
mcpe_viz.xml -> data/mcpe_viz.xml
mcpe_viz.html.template -> static/mcpe_viz.html.template
Feel free to email me if you need any help, this is a pretty big update. Thank you

@tomnolan
Copy link
Member

@paulgrahek I think there was a comment in the issue thread from @picotazo that they put it into the root folder to have it be used by the app when it executes since the data folder isn't searched by the app. I believe that is intentional, correct?

@paulgrahek
Copy link
Member

@tomnolan good catch. When this repo was forked some of the configuration files were moved when the project was restructured, but it does not appear the code was updated to look for the configuration file in the new data folder. Currently the folder looks in the command line, the home directory (is this one necessary?), and then the local folder of the application.
The build configuration does copy the gui and data folders into the build directory, so it appears the new intended behavior is to look for the configuration file in the data folder.
@jasper-wan what do you think?

@scp-r
Copy link
Contributor

scp-r commented Mar 12, 2020

@paulgrahek you are right, there is a bug in load_cfg function and we need to add <install-prefix>/data/mcpe_viz.cfg to the search location of cfg file. Since users can not change files in /usr/local/share, the cfg in this folder may be treated as a default configuration if no configuration file is found.
New files in root directory of the project won't work unless you modify CMakeLists.txt to process it . Currently, cmake will copy specific files to destination folder after building the program:

# the data folder
file(COPY data
     DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
# the static folder
file(COPY static
        DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
# we dont need to `make install` on Windows
install(TARGETS bedrock-viz DESTINATION bin) # program itself
install(DIRECTORY data/ DESTINATION share/bedrock-viz/data) # data
install(DIRECTORY static/ DESTINATION share/bedrock-viz/static) # static 

@tomnolan
Copy link
Member

I ran through this xml file last night and there are a bunch of edits needed to make it work. I'm going to make an update for this PR this weekend with those sites and I'm trying to get some of the new nether update blocks included as well. Then we should be able to merge this one as well.

@picotazo
Copy link
Author

I looked through it last night and found I had a bunch of Block ID's that were all off by a digit. Sorry about that.

@tomnolan
Copy link
Member

@picotazo I'm going to close this PR since it no longer applies. Thank you for getting the XML for this started though, I made sure to mention you in the changelog :-)

@tomnolan tomnolan closed this Jun 29, 2020
@picotazo
Copy link
Author

picotazo commented Jun 29, 2020 via email

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

4 participants