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

Z-level update: lots of overloads #11705

Merged
merged 21 commits into from Mar 23, 2015

Conversation

Projects
None yet
5 participants
@Coolthulhu
Copy link
Contributor

commented Mar 21, 2015

Quite a bit of overloads. No in-game effect yet.

Includes #11472

Doesn't yet implement everything in #6818 but is close.

@Lain-

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2015

Just stating the obvious here, but I hope this get merged soon, because 1) It's awesome and 2) A big PR like this tend to get unmergeable very quick.

Merge branch 'master' into zlev-visibility-basic
Conflicts:
	src/advanced_inv.cpp
	src/game.cpp
	src/player.cpp
	src/savegame_json.cpp
@Zireael07

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2015

Agreed!

@@ -8668,7 +8668,8 @@ point game::look_around(WINDOW *w_info, const point pairCoordsFirst)
const int offset_x = (u.posx() + u.view_offset_x) - getmaxx(w_terrain) / 2;
const int offset_y = (u.posy() + u.view_offset_y) - getmaxy(w_terrain) / 2;

int lx = u.posx() + u.view_offset_x, ly = u.posy() + u.view_offset_y;
int lx = u.posx() + u.view_offset_x, ly = u.posy() + u.view_offset_y, lz = u.posz() + 0;
tripoint lp( lx, ly, lz );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 21, 2015

Contributor

You could avoid the overriding of lp below by using this neat reference trick:

tripoint lp( u.posx() + u.view_offset_x, ... );
int &lx = lp.x;
int &ly = lp.y;
int &lz = lp.z;

If you write to lx, it will automatically update lp because lx and lp.x are the same place in memory.

src/map.cpp Outdated
return it->second.first;
}

debugmsg( "vehicle part cache cache indicated vehicle not found: %d %d %d", p.x, p.y, p.z );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 21, 2015

Contributor

"cache cache"? Is that something like a cache of a cache, it caches caches? (-:

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Mar 21, 2015

Author Contributor

So you can lookup your lookups

src/map.cpp Outdated
nulfield = field();
return nulfield;
}

int lx, ly;
submap *const current_submap = get_submap_at( x, y, lx, ly );
submap *const current_submap = get_submap_at( p.x, p.y, p.z, lx, ly );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 21, 2015

Contributor

Why not get_submap_at( p, lx, ly )?

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Mar 21, 2015

Author Contributor

Had a bit of "branching hell" there for a while. Some of the changes are from earlier branches (before I created some of the useful overloads).

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2015

This looks fine.


Does not really belong in this PR, just some things I noticed (should probably addressed, if at all, after this has been merged to prevent conflicts):

Have you noticed that map::has_flag_ter_and_furn is implemented as ter_at.has_flag && furn_at.has_flag while map::has_flag_ter_or_furn is implemented quite differently (look up of the submap only once)? That makes no sense, both function should be the same except for the single || vs && when combining the flag status.

Having the look up of the submap and the calculation of the submap local coordinates lx, ly (and checks for inbounds only once) should be a bit faster than doing it twice.


map::tername, map::furnname and map::name should probably return a const reference.

Similar some functions that take a furniture / terrain / trap id string (e.g. map::set) should take that string as const reference.


This might reduce the code size a bit: make get_submap_at check inbounds itself and return a nullptr if the input is not inbounds. That way the call can avoid that inbounds check and one should probably check the return value get_submap_at anyway. Instead of:

    if( !inbounds( p ) ) {
        return false; // or whatever
    }
    int lx, ly;
    submap *const current_submap = get_submap_at( p, lx, ly );

One can write:

    int lx, ly;
    submap *const current_submap = get_submap_at( p, lx, ly );
    if( current_submap == nullptr ) {
        return false; // or whatever
    }

Nah, probably not a good idea.

@Zireael07

This comment has been minimized.

Copy link
Contributor

commented Mar 22, 2015

Merge please before it gets un-mergeable...

@kevingranade kevingranade merged commit 50c9310 into CleverRaven:master Mar 23, 2015

1 check passed

default
Details

@Coolthulhu Coolthulhu deleted the cataclysmbnteam:zlev-visibility-basic branch Apr 10, 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.