cayrusa / SA-map-editor

0 stars 1 forks source link

Display edition Of tiles in map editor #1

Closed LokiMidgard closed 2 years ago

LokiMidgard commented 2 years ago

My proposal would be to add a edition property in the tiles.json manifest

{
    "path":"assets/tiles/Redux/",
    "manifest": [
        {"id":"Tile-Abyz-Fria", "src":"AbyzFria.png", "edition": "TI3", "planets": [
            {"name":"Abyz"}
            ,{"name":"Fria"}
        ]},
                //...
        ]
}

I managed to render it in the tiles panel

image

The current implementation shows the edition if set, and SA otherwise. So every tile not belonging to an edition would automaticly be a SA exclusive tile.

It should also be possible to display add all edditions required for the current map by iterating over all tiles used.

The editions used in the map will be displayed in the top left of the map defiend by the most left and the most top tile.

map (1) (Mittel) will be displayed in the top left of the map defiend by the most left and the most top tile.

What I don't know is how the file TI3SA-0.1.js gets generated it looks like a minified version of some other files, but I havn't seen documentation how to generate it. Thats why this pull request is not correctly working. I have added the minified version localy to reproduce my changes to veryfiy them. But I think it is unrewarding to commit those changes.
See comment bellow

I also hope this is the correct version, since it looks a bit different then the deployed one.

cayrusa commented 2 years ago

I don't have the original, unminified, TI3SA-0.1.js file. I'll ask the original creator, but no guarantee.

I also hope this is the correct version, since it looks a bit different then the deployed one.

How does it differ?

cayrusa commented 2 years ago

I asked the original creator (Tzeen). This minified file is actually the result of merging and minifying all the other js files. He thinks the minifier was uglifyJS.

cayrusa commented 2 years ago

Additional lead from the creator on where to find the code responsible for that feature:

panelcontainer does the populating of the tiles from the manifest files, so that would be a good place to start just look at the constructor method and follow the trail from there

LokiMidgard commented 2 years ago

I don't have the original, unminified, TI3SA-0.1.js file. I'll ask the original creator, but no guarantee.

I also hope this is the correct version, since it looks a bit different then the deployed one.

How does it differ?

Ok I was lazy copying the text over... the version I originally started with had some more tokens ;)

I asked the original creator (Tzeen). This minified file is actually the result of merging and minifying all the other js files. He thinks the minifier was uglifyJS.

I'm not sure if I get to it this week. But it should not be to hard. Would you mind if I would add an package.json of npm to automate the minification?

LokiMidgard commented 2 years ago

Ok, I had some time in the bus on the way to work...

I added a package.json if you have node installed you can run npm i npm run build and you should get a minfied version in the dist folder. It will also download the assets from your website and extract those in the dist folder.

I have also found a place to calculate the tiles used in the map. But I'm not yet sure where to put the Informatin on the screen...

For now I just log it to the console.

cayrusa commented 2 years ago

I am not an expert in npm. I tried to run npm run build, but got an error: image Is it on my end? Is it on yours?

cayrusa commented 2 years ago

Ok, I was doing as your new readme said (just npm run build), and not as your comment said (npm i beforehand). Doing as your comment said works for me.

cayrusa commented 2 years ago

npm run dev gave that error: image

LokiMidgard commented 2 years ago

I updated the readme, somtimes it is hard to remember what you need to do if you do it automaticly...

I just saw that the serv project I used is no longer maintained, I replaced it with serve. I hope that will work on your machine.

From the error I think it is the console your using, I use windows Terminal. The project I used propably did not handle different shells well. I have hopes that the new one will work better, If not please post the error again (if it looks different)

cayrusa commented 2 years ago

Tried again. Didn't work. I get that "serve" is not found when I npm run dev. By the way, npm run install complained that the "install" script is missing.

I usually dev on linux, but at the moment I have a windows. I'm not used to (and kinda confused by) windows command lines. I tried the above (and got similar results) with the "git bash for windows" and the linux subsystem for windows (ubuntu).

LokiMidgard commented 2 years ago

I fixed the npm run install in the readme it is only npm install. Damn Copy and Past.

Ive you did not run npm install after the pull serve will not be installed, sorry for the confusion.

I copy paste what I type from the command line, that way I should not be able to mess up:

This how it looks in my console (not quite, since the browser does not displays all chars correctly...):

╭─    ~                                                                                        ✔  11:39:01  ─╮
╰─ cd .\source\repos\SA-map-editor

╭─    ~\source\repos\SA-map-editor   master                                      16.13.1   ✔  11:39:05  ─╮
╰─ npm i

up to date, audited 116 packages in 2s

11 packages are looking for funding
  run `npm fund` for details

1 moderate severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

╭─    ~\source\repos\SA-map-editor   master                           16.13.1   2.522s   ✔  11:39:09  ─╮
╰─ npm run build

> sa-map-editor@0.1.0 build
> node scripts/pack.mjs

extracting assets...
assets extracted
minifiying...
minified
copy local assets
copy index.html
Finish

╭─    ~\source\repos\SA-map-editor   master                             16.13.1   6.6s   ✔  11:39:25  ─╮
╰─ npm run dev

> sa-map-editor@0.1.0 dev
> serve dist

   ┌────────────────────────────────────────────────────┐
   │                                                    │
   │   Serving!                                         │
   │                                                    │
   │   - Local:            http://localhost:3000        │
   │   - On Your Network:  http://192.168.178.46:3000   │
   │                                                    │
   │   Copied local address to clipboard!               │
   │                                                    │
   └────────────────────────────────────────────────────┘

PS:
If your working on windows I can highly recommand the new terminal. It can host different consoles like powershell, cmd and I think also gitbash. It has a much better support for things like colors and copy paste

LokiMidgard commented 2 years ago

I now added the used editions to the map. This way it will also be printed to the exported image.

If this is ok, the only thing that now needs to be done is adding editions to all tiles.

LokiMidgard commented 2 years ago

While I hope (and think) localy serving the files should work now, I temporaryl hosted this version of the editor on heroku.

It's a litle bit slow when loding the assets, but it should work.

cayrusa commented 2 years ago

Ok. It works on my machine now. Great! I'm looking through the pull request to understand what's happening. I'm not familiar with javascript development so I'll probably ask a few questions.

cayrusa commented 2 years ago

I finished reading, and asked a few questions along the way to understand better. Overall: good stuff. Thanks! Having an easy deployment system is going to be nice.

Now as to the feature itself:

LokiMidgard commented 2 years ago

I updated some stuff based on your feedback

I removed the not yet used dependencys and the directory entry.

I also added a UI in the html. But I don't like it. I'm not sure how to display it.

If you had something in mind feel free to correct it. The JS searchs an element with the id tileset-list and adds li tags for every edition. It will also change the visibility of the firest legend tag found to none when no entry is in the tileset-list

LokiMidgard commented 2 years ago

I added the metadata for TI3 and TI3:SE that were the tiles I had currently on my desk. besiedes the edition I also added everything interresting about the planet like influence wormholes sience...

Maybe in the future someone want's to add filter options...

I also added a schema, so an editor that support it will have autocompletion. Every additional metadata is currently optional.

LokiMidgard commented 2 years ago

I added the Homesystem placehoders to TI3 thats not correct, but otherwies it would show SA as required tileset.

We could change it so tiles that have no tileset will not default to SA but get ignored....

cayrusa commented 2 years ago

I added the Homesystem placehoders to TI3 thats not correct, but otherwies it would show SA as required tileset.

We could change it so tiles that have no tileset will not default to SA but get ignored....

Good idea.

cayrusa commented 2 years ago

Windows terminal is nice. Thanks for the link! Other tool question: what do you use as editor for js?

cayrusa commented 2 years ago

About redux.schema.json, I understand it's a mean of validating the format of the tile manifest?

LokiMidgard commented 2 years ago

Windows terminal is nice. Thanks for the link! Other tool question: what do you use as editor for js?

I use VSCode

cayrusa commented 2 years ago

About redux.schema.json, I understand it's a mean of validating the format of the tile manifest?

Yeah ok, you answered already. My bad. ("I also added a schema, so an editor that support it will have autocompletion. Every additional metadata is currently optional.")

cayrusa commented 2 years ago

I'll add the data for the rest of the tiles

LokiMidgard commented 2 years ago

About redux.schema.json, I understand it's a mean of validating the format of the tile manifest?

yes, you can do that. but the main reason was so I get autocompletion when setting the editions. That prevent typos and if there would be one the editor shows a red mark were the error is. It is not used to validate the data at runtime.

Its currently pretty lax, if the data, like edition is complete, it can add more required fields. So future changes cold be validated

cayrusa commented 2 years ago

I just noticed there's an issue with the exported images: they lack background and padding

Image exported by this editor: image

Image exported before these changes: image

cayrusa commented 2 years ago

ok, so what's missing before I can merge this pr and deploy on astralvault: a) Fixing the exported image not having background and padding b) Tiles with no tileset shouldn't be considered as "SA" by default, they should be ignored from the tilesetlist computation (and SA tiles should have their tileset explicitly set to "SA" in the manifest) c) Filling in data for the rest of the tiles d) Shrinking the legend showing the required tileset. It does not deserve all the space it currently use. I suggest changing its title to just "Tilesets" and removing some vertical padding. Ideally, it should be as compact and as "out of the way" as possible.

cayrusa commented 2 years ago

Two problems for the future (i.e. let's merge this pr first, and worry about them later): 1) Domain Counters should also have an edition 2) Since the idea is to show if a map is buildable with only this or that edition, it should also take into account the number of tiles used for special system. For an extreme example, a map with 25 Asteroid Field tiles is fine for TTS, but unbuildable in real life. And same thing for the tokens: there is a finite amount of them in the boxes.

cayrusa commented 2 years ago

ok, so what's missing before I can merge this pr and deploy on astralvault: a) Fixing the exported image not having background and padding b) Tiles with no tileset shouldn't be considered as "SA" by default, they should be ignored from the tilesetlist computation (and SA tiles should have their tileset explicitly set to "SA" in the manifest) c) Filling in data for the rest of the tiles d) Shrinking the legend showing the required tileset. It does not deserve all the space it currently use. I suggest changing its title to just "Tilesets" and removing some vertical padding. Ideally, it should be as compact and as "out of the way" as possible.

If you see anything else, feel free to add to that list.

cayrusa commented 2 years ago

It goes without saying but it goes better when said: thanks for all your good work!

cayrusa commented 2 years ago

I added all the tile data. In the process, I noticed the border tokens (asteroid belt, ion sphere...) are declared at the end of the tiles manifest and not in the tokens manifest. I have no idea why. I guess the editor works regardless of that because it uses the first part of the id tag (in this case "Token" for "Token-AsteroidBelt") to sort out which asset goes in which panel.

LokiMidgard commented 2 years ago

I think I got all your change requests

I also changed it that theoreticly every map element can now have an edition, so it can be added to the tokens. I also like the idea of having a component count.

Depending on the number of tiles in the expansions the logic could be interresting. If both expansions would add an extra astoroid belt and you need one more then in the base game, you need EITHER expansion, but adding a gravity rift would then require only SotT since gravity rift is only available there...

This would be fun I guess 🤔

cayrusa commented 2 years ago

Yeah there will probably be a bit of thinking required. I'll grab my box and count the tiles so we have the actual numbers.

I don't see any more issues right now. I'll merge this pr and deploy. I'll make an issue about the tile numbers (with the data I'll get from counting my b ox).