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

Parse the zipped files into our representation of objects. #529

Closed
dtbukowski opened this issue May 11, 2020 · 15 comments
Closed

Parse the zipped files into our representation of objects. #529

dtbukowski opened this issue May 11, 2020 · 15 comments
Assignees
Labels
Milestone

Comments

@dtbukowski
Copy link
Contributor

dtbukowski commented May 11, 2020

Communicate with Nam with how the .wdz files look, then parse those files into the representation of objects that blake and noah decided on.

@dtbukowski dtbukowski self-assigned this May 11, 2020
@dtbukowski dtbukowski added this to the 2020/Sprint 3 milestone May 11, 2020
@dtbukowski
Copy link
Contributor Author

Made progress on the current state of libzip in the CS Machines with a commit on wdl/parsing_zipped (thought the branch doesn't show up and I may need to push it). Although share objects exist in repositories /usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/libzip.so/usr/lib/jvm/java-11-openjdk-amd64/lib/libzip.so
/usr/lib/vlc/plugins/access/libzip_plugin.so, I cannot find the zip.h file necessary to complete. There is either a mistake in my Makefile or libzip needs to be installed in usr/lib/x86_64-linux-gnu, so I will use zziplib for now and try asking techstaff to download libzip

@dtbukowski
Copy link
Contributor Author

Was able to successfully open and close a zip file since I finally had the right libraries for zip.h and the header installed in the Virtual Machine. Libzip isn't installed on the CS machines so as of now I am not able to use libzip on the CS machines.

Will push my progress once I am able to fix errors pushing onto Github. Apparently there are errors with remotes.

@dtbukowski
Copy link
Contributor Author

Pushed progress on using libzip where I figured out how to get file indexes and get files from name (has to be specific directory) or index.

Next I will use Noah's implementation to make JSON objects and push that onto master

@dtbukowski
Copy link
Contributor Author

I have pushed code that will make JSON objects, but specifically only for the 5 JSON files in the sandbox in wdl in parsed_zipped directory.

However, the errors present are because of JSON not being present in the VMs so I will either install JSON myself tonight or, depending on how fast techstaff acts, use my own computer with libzip installed.

I have implemented a way to loop through all files in a zip directory, however I will need to figure out how to:
Get files that end in .json (check if filename ends in .json)
Get files that are images and files that are sounds (looked into it here: https://stackoverflow.com/questions/670546/determine-if-file-is-an-image)
Loop through all JSON objects in a .json file (current implementation of json_parse_test that uses the name of the field will not work because new field will require this file to constantly be updating.)

@dtbukowski
Copy link
Contributor Author

@dtbukowski
Copy link
Contributor Author

Pushed Pseudocode that I will work on tomorrow for parsing zip files into general objects. I will loop through the zip files and in any json files I will loop through them and add the key value pairs to the general object

@dtbukowski
Copy link
Contributor Author

Turned most of Pseudocode into real code. However, the only parts I was able to test right now are to see if the functions that parsed directories as a string (like game/game/actions.json) was able to parse correctly. If we are able to install libzip soon then we should use the CS machnes. Otherwise anyone who wants to test will need to install json-c and libzip on a VM.

Things done:
Added onto the code right before the loop and added assert to the functions that use
makejsonobj to see if the function works
Code to get objtype from json file name.
Code to see if a file name is a json file.
Code to get specific types of json objects such as string or int to add to file.

Still need to get done:
TEST the file! (Install libzip and json-c on Virtual Machine if we have to wait a long time for libzip)
When objtype is object, currently has json_object and this needs to be converted into libobj. (lines 143-146)
Add necessary header files to parsetojsonobj - it is called parsetojsonobj even though it
parses to libobj implementation so either the name will be changed or to split the file into multiple
files.
Add a method to get assets associated with the .json files.

@dtbukowski
Copy link
Contributor Author

Moving this issue to Sprint 4 since this issue wasn't complete by Sprint 3. Also assigning Nam to this issue since this is something we need to get done for teams to test their code.

@namanhd
Copy link
Contributor

namanhd commented May 30, 2020

I tried to use @dtbukowski's wdl/parsing_zipped branch from last sprint but it looks like the wdz files in this branch have been moved into the src directory which is potentially messy for when I eventually merge #709 which moves the test wdz into wdl/examples/wdz. Since I will also be adding this drop-in zip library into the source and include directories, I have decided to make a new branch wdl/wdz-to-json (out from wdl/move-wdz-to-examples) so that I can have the latest wdz test material in the right folder and safely include the new zip library code and headers.

As another note, I've decided to use this zip library because right now as a backup option in case it takes a while to get libzip onto the CSIL machines (which chiventure is exclusively tested and run on), and it is far more convenient to simply bundle in this lightweight and simple alternative, which is also under the Unlicense.

@namanhd
Copy link
Contributor

namanhd commented May 31, 2020

An update to the zip library situation: we were advised a workaround to have libzip available for building chiventure on the CSIL machines (in the wdl/parsing_zipped-cmake branch), which means I can now experiment with libzip proper, instead of the drop-in miniz-based zip library. As such, I have renamed the branch I created in the previous update comment (wdl/parsing-wdz-no-libzip) to just wdl/parsing-wdz-namanh where I will be testing the zip-opening features using both libraries and attempting some early integration into the existing game loading interface.

@namanhd
Copy link
Contributor

namanhd commented Jun 1, 2020

I have managed to patch the wdz loading feature into the current WDL, albeit in a really rudimentary way. We can now pass a wdz file into the chiventure executable, and instead of loading up the main UI and game, chiventure will print out the contents of all JSON files in the wdz archive to confirm that it can read the archive.
I suspect that this entrypoint (in the old wdl load_game function) will also be used when we integrate our final implementation with the object store and everything else.

@dtbukowski
Copy link
Contributor Author

I managed to set up my parsing_zipped branch with cmake and got the code to compile without issues.

@namanhd
Copy link
Contributor

namanhd commented Jun 8, 2020

Successfully merged a branch that contains the zip loading functionality and a way to check if the JSON files have been read. See #800.

Functionality for actually turning those json objects into internal game objects and loading them into our object store is pending, and might become a backlog issue for later.

@namanhd
Copy link
Contributor

namanhd commented Jun 9, 2020

It looks like #806 is refusing to pass on the csil machines no matter what I try; the program does not break when run in valgrind, and gdb traces look identical to what they are on a working, non-crashing build on my personal machine; furthermore, the latest merges to dev (with wdl_ctx) have also conflicted with this request. Therefore, this should receive more attention in a separate (possibly backlog) issue, and we are closing this one which did result in a successful PR with the zip file loading.

@namanhd namanhd closed this as completed Jun 9, 2020
@MaxineK36
Copy link
Contributor

Issue Score: ✔️+

Comments:
Good job! Here a few things to improve in future issues:

  1. Even though you can add labels to issues to categorize them, you should avoid titles that could plausibly apply to multiple teams. The title would have been clearer if you'd specified that you were parsing .wdz files into wdl objects
  2. The summary for this issue is too terse, and refers cryptically to conversations that other developers wouldn't have been part of, which makes it hard for someone else to figure out what you were trying to get done here.

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

No branches or pull requests

3 participants