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

Isometric tile view [WIP] [CR] #11812

Merged
merged 30 commits into from Apr 27, 2015

Conversation

Projects
None yet
10 participants
@sparr
Copy link
Member

commented Mar 26, 2015

First commit of code changes are very small. Just modified tile placement and an isometric flag. This needs to be extended to include loading rotated tiles instead of doing tile rotation in SDL. The tile drawing order also needs to change, including separate passes for FG and BG tiles.

This PR includes a script to naively convert an existing tileset to an equivalent isometric tileset. It's got a lot of magic numbers and strings in it right now that make it only work with ChestHoleTileset.

screenshot:
isometric screenshot

I'll be poking at this idea more in the near future.

@@ -0,0 +1,76 @@
# creates tileset_iso/ based on tileset/

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 26, 2015

Contributor

What interpreted is supposed to run this script? There is no shebang!

This comment has been minimized.

Copy link
@sparr

sparr Mar 26, 2015

Author Member

Just pick an interpreter at random. It's sure to work in at least one of them :)

(this script will be replaced with something in perl or python or another language that understands json natively, because it's going to need to do json modifications, including making new entries)

@@ -270,6 +270,11 @@ void cata_tiles::load_tilejson_from_file(std::ifstream &f, const std::string &im
JsonObject curr_info = info.next_object();
tile_height = curr_info.get_int("height");
tile_width = curr_info.get_int("width");
if (curr_info.has_bool("iso")) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 26, 2015

Contributor

x = json.get_bool( "...", false ) The second parameter to the various get_* functions is the default value.

This comment has been minimized.

Copy link
@sparr

sparr Mar 26, 2015

Author Member

I saw a JsonObject.get_something() cause the whole json loading process to fail when something was missing. We should probably make use of those defaults in more places...

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 27, 2015

Contributor

Defaults can also be dangerous as the program can not detect spelling errors that way. They are useful if there is a meaningful default (e.g. a value that is used in nearly all situations), or (like here) for backwards compatibility.

But this is a different discussion, my comment was about syntax: if you have a default value, use the json function that will automatically use them. Don't repeat the "if value does not exist, return default" code, which is already part of the json system. This is also easier to read.

@Chezzo

This comment has been minimized.

Copy link

commented on 8e01ad6 Mar 27, 2015

Hey Sparr, remember you were asking if 24x32 is a valid tile size? This link is about isometric math, not sure if it is already applied.

http://clintbellanger.net/articles/isometric_math/

It probably is, you are a smart guy.

Anyways, I love this and am working furiously to make it look sharp. Thanks so much.

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2015

To make this work, tile_config.json is going to need to be extended to allow rotated tiles to be specified.

I'm thinking that anywhere 'fg' and 'bg' currently exist, we could insert a rotated-sprites list:

"fg": 1000,
"bg": 2000,

becomes

"fg": 1000,
"bg": 2000,
"rotated-sprites", {
        "north": {"fg": 1000, "bg":2000},
        "east":  {"fg": 1001, "bg":2000},
        "south": {"fg": 1002, "bg":2000},
        "west":  {"fg": 1003, "bg":2000},
    },

or maybe rotated-sprites could just be an array of 4 (or 8) elements without direction labels?

"fg": 1000,
"bg": 2000,
"rotated-sprites", [
        {"fg": 1000, "bg":2000},
        {"fg": 1001, "bg":2000},
        {"fg": 1002, "bg":2000},
        {"fg": 1003, "bg":2000},
    ],

Or maybe fg and bg could themselves be arrays?

"fg": [ 1000, 1001, 1002, 1003 ],
"bg": 2000,

With all of the un-labeled array options, I think we'd specify that 1 entry can be a number or a 1-entry array, and represents a tile that doesn't rotate (like a tree sprite). A 2-entry array would mean the first is 0 or 180 degrees and the second 90 or 270, useful for things like straight walls that look the same when flipped. 4 entries would give the normal 4 rotations, for things like T-walls, wall endcaps, etc. Allowing more than 4 entries would let us specify partially rotated vehicle parts, which may be slightly desirable.

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 27, 2015

I chose the simple array in implementing #11823, discussion about that feature should happen over there. This PR will not land without that one, or one like it.

@Chezzo

This comment has been minimized.

Copy link
Contributor

commented on cceb4a3 Mar 28, 2015

Sparr, I cannot for the life of me get foregrounds and backgrounds to rotate at the same time. It's fine, I'll just put 'em together in Gimp.

This comment has been minimized.

Copy link
Contributor

replied Mar 28, 2015

Oh no! There's a lot of whitespace in these doors, and if I fill it with the outside walls, they will look terrible downstairs.

This comment has been minimized.

Copy link
Member Author

replied Mar 28, 2015

I don't think BGs can rotate, right now? That wasn't my decision, pre-existing code. I'll investigate later.

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2015

tiles outside the view distance aren't getting overdrawn with blank tiles, so there's a lot of leftover drawn tiles out there. I need to fix that. Otherwise I think this is looking pretty good. Chezzo is possibly interested in drawing a set of isometric tiles. He's already started playing with the idea of zelda-style 3d with the manual rotation feature in an "overhead" view.

to accommodate zelda-style and isometric, I think maybe the tileset definition needs separate parameters for the height of a sprite and the height of a drawn map tile.

also, if we start letting tiles overlap in any way, we're going to need to draw all the tiles in a certain order (terrain bg, furniture bg, terrain fg, furniture fg, etc) instead of just drawing one square at a time.

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2015

one thing I haven't put into make_iso.py yet is handling of fallback ascii tiles.

also, I need to put in a list of tiles that should be handled specially, such as the "pile of items in this square" indicator (highlight_item?).

more ambitiously, I'd like to get make_iso.py to read the non-tileset json files, to get information about the terrain, furniture, items, etc, then use this information in deciding how to adapt the tiles. for example, impassable terrain like walls should be drawn with some "height" to its sprite. along with the ability add height to sprites would come non-flat vehicles, etc.

of course, make_iso.py is a stop gap; eventually if we put isometric mode in the core game we'll have real sprite artists making isometric sprite art.

current screenshot:
isometric screenshot

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 29, 2015

Also, I haven't done anything with mouse handling yet, so mouse hover and clicks still get interpreted on the square grid.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Mar 29, 2015

This looks really great!

@Dozed12

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2015

0_0 How? When? This is so awesome!

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2015

Basic (non-isometric) tile mode can certainly draw from any direction, tiles don't overlap there. Beside the isometric view, is there any other case you need it?

The nature of the loop can change.

In that case there is exactly no overlap between drawing rectangle tiles and isometric tiles. The loop is different, the content is different (rectangle tiles need one loop only, isometric need several, rect can often stop after light check, etc.).


Have you ever heard of YAGNI? Also: refactoring is fun (-:

@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2015

zelda-perspective tiles will need to draw north to south.

square tiles that are bigger than the grid will need to draw one layer at a time, like isometric.

both of these are features that I plan to add, and that will benefit from improvements made while implementing isometric mode.

@Kurruk007

This comment has been minimized.

Copy link

commented Mar 31, 2015

Sorry to bump in uninvited... but:

  • breaking drawing into separate loops will cause issues when more objects will be higher than 1 tile (like walls) they will be overdrawn by ALL consecutive elements
  • diamond painter approach assumes drawing with two loops: outer (top-left) and inner (top-right) ... so when rotating the view you would have to rework loops for both min-max and x-y in order to always draw top to bottom
  • multiplication in loop (condition) would have to be pre-calculated regardless if you want to rotate or not
@sparr

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2015

Good point, re tall tiles. I think that means that if we have tall-and-wide tiles then we have to draw in rows, one layer at a time.

so, row 1 terrain, row 1 furniture, row 1 monsters... row 2 terrain, row 2 furniture...

@Kurruk007

This comment has been minimized.

Copy link

commented Mar 31, 2015

When assuming your row is the top-left line, then yes. When drawing that one and then moving to the row below it will naturally be covered by tall object. Other alternative that I used way back was zbuffer, but in order for it to work 3d would be required either via element or faking it depending on object... but that is part of a different idea I had regarding cdda, which I might write down at some stage.

Asking uncle google, I've also found another option, which is zig-zag:
http://stackoverflow.com/questions/892811/drawing-isometric-game-worlds

@Chezzo

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2015

we'll have real sprite artists making isometric sprite art.

Where are we gonna get one of those?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 1, 2015

Making a generator for the points and then iterating over that would be a viable approach, but I feel like that approach incurs more non-optimzer-friendly overhead.

Try it out and profile it, I'm skeptical that doing the exact same loop once and loading the points into a lightweight container, then iterating over that container repeatedly is going to be a problem. Hell, if the bounds are constant, you can calculate the size needed and load them into an array, then still use the range iterator. The generate/iterate approach has the added benefit of clearly separating the iteration order from the code iterating over it, you can trivially change one without affecting the other.


for (auto& fg : curr_subtile->fg) {
if (fg < 0 || fg >= size) {
entry.throw_error("invalid value for fg (out of range)", "fg");

This comment has been minimized.

Copy link
@kevingranade

kevingranade Apr 8, 2015

Member

Need to either tolerate a value of -1 to indicate no background or foreground, or update all the tilesets to remove their negative values, a lot of them fail to load now.

This comment has been minimized.

Copy link
@sparr

sparr Apr 11, 2015

Author Member

fixed this by not loading -1s. what used to be fg=-1 will now be fg=[] which is the same no-sprite empty-array as not specifying the fg at all.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 8, 2015

The only other issue is http://polehammer.com/azmodean/tile_seams.png, it looks like the background tiles are being rotated and it's causing misalignment. I'm wondering if that's why they were being prevented from rotating. I suspect your heuristic for detecting that a square isn't intended to be rotated is off, I think some backgrounds have alternates to break u the tiling effect.

@DavidKeaton

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2015

Why not abstract those loops to a function, with arguments of function type and a new overloaded method to take int vector for each?

@stk2008

This comment has been minimized.

Copy link

commented Apr 8, 2015

I really like the look of this :)

@sparr

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2015

@kevingranade I am unable to reproduce the alignment problem you're seeing. It looks like the tiles in question are just grass and dirt. Can you post a save that demonstrates it?

@sparr

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2015

I can't get input_context::get_coordinates to work quite right in isometric mode. I've just committed a patch that is close, but very hack-y and not nearly accurate enough. I'm hopeful that someone more familiar with the tiles drawing code (particularly the part where window position and size are in different coordinate systems) can help out here.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 14, 2015

Not sure what happened, now the process suffers a very hard lockup on game start, I strongly suspect during tile loading.

@kevingranade kevingranade merged commit 5c3b529 into CleverRaven:master Apr 27, 2015

1 check failed

default Unmergeable pull request.
@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 27, 2015

The issue with the hard lock was a stack overflow, if a tileset is missing the "unknown" tile it recurses infinitely.

@sparr

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2015

oh, hmm. and my chesthole iso tileset didn't have an unknown tile?

@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 28, 2015

@DavidKeaton

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2015

where we're going, we don't need stack space.

puts on glasses and jets to puts on glasses and jumps to puts on glasses and jumps to

@sparr sparr deleted the sparr:isometric_tiles branch Nov 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.