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

Merge all `*State` structs and `GameState` trait into one universal `State` struct #255

Closed
ozkriff opened this Issue Dec 28, 2016 · 3 comments

Comments

1 participant
@ozkriff
Owner

ozkriff commented Dec 28, 2016

Required for #253 and must greatly simplify codebase.

Right now I have InternalState, FullState, PartialState, TmpPartialState structs and GameState trait which is implemented for all of them. This greatly complicates #253 (see comments) because you need generalized code in some places and concrete struct in other places + it introduses a few lifetime-related problems with TmpPartialState.

So I want to try to merge all this into one universal State struct which can fill all these roles and will not have any lifetime parameters.

The only way to achieve this that I see is to use "option hack": have an for: Option<Fow> fields in the struct and insert/extract (using Option::take and movement semantics) it for switching between FullState/PartialState roles.

Demo: https://play.rust-lang.org/?gist=401412510e667a291c151b84b7be6a98

@ozkriff

This comment has been minimized.

Show comment
Hide comment
@ozkriff

ozkriff Dec 28, 2016

Owner

Working in WIP_255_state branch

Owner

ozkriff commented Dec 28, 2016

Working in WIP_255_state branch

@ozkriff

This comment has been minimized.

Show comment
Hide comment
@ozkriff

ozkriff Dec 29, 2016

Owner

Almost finished - it works the same as before but with unified State struct.

The thing I want to fix now is that I can't merge State and Fow in src/player_info.rs because of filtering problems.

First comes the EndTurn event, then HideUnit events (if any enemy's units become unseen). The problem is that State with Fow filters out all units that are hidden by Fow so I can't get enemy's unit from State during the execution of HideUnit event.

I think that the least bad solution is to redo handling of EndTurn in filter.rs to insert HideUnit events before EndTurn.

But I'm not sure what exactly i need to do for this. Right now the filtering system assumes that Fow is already updated. Should I somehow reorder events (remeber EndTurn events in some place and append it after all HideUnit events)? Hmmmm. That needs an investigation

Owner

ozkriff commented Dec 29, 2016

Almost finished - it works the same as before but with unified State struct.

The thing I want to fix now is that I can't merge State and Fow in src/player_info.rs because of filtering problems.

First comes the EndTurn event, then HideUnit events (if any enemy's units become unseen). The problem is that State with Fow filters out all units that are hidden by Fow so I can't get enemy's unit from State during the execution of HideUnit event.

I think that the least bad solution is to redo handling of EndTurn in filter.rs to insert HideUnit events before EndTurn.

But I'm not sure what exactly i need to do for this. Right now the filtering system assumes that Fow is already updated. Should I somehow reorder events (remeber EndTurn events in some place and append it after all HideUnit events)? Hmmmm. That needs an investigation

ozkriff added a commit that referenced this issue Jan 2, 2017

@ozkriff

This comment has been minimized.

Show comment
Hide comment
@ozkriff

ozkriff Jan 6, 2017

Owner

That was really silly: Fow in player_info.rs is not Fow from fow.rs, it's a distinct helper struct for smooth animation of FoW updates. Renamed it to FowInfo just in case: 7cc82ff .

I had some problems with ShowUnit that I was not able to solve with reordering of EndTurn and ShowUnit/HideUnit - when enemy comes out of the fog first comes ShowUnit event for enemy's pos in the fog, then comes MoveUnit and its handler crashes as State filters out this new unit. Fixed this problem by adding shown_unit_ids: HashSet<UnitId> field to State struct: all UnitIds from ShowUnit events go there and these Ids are not filtered out even if their Unit is in the fog:

fn is_unit_visible(&self, unit: &Unit) -> bool {
    let fow = match self.fow {
        Some(ref fow) => fow,
        None => return true,
    };
    fow.is_visible(unit) || self.shown_unit_ids.contains(&unit.id) // <-
}

Plus, there was a problem with EventHideUnitVisualizer constructor - I can easily delete the node itself (by getting its Id from scene.unit_id_to_node_id) but I can't ask unit's MapPos for creating pop up text from State anymore. So I've extracted from pick.rs and employed world_pos_to_map_pos function: ad0e956 .

let world_pos = scene.node(node_id).pos;
let map_pos = geom::world_pos_to_map_pos(world_pos);
map_text.add_text(map_pos, "lost");

Added some basic maps for testing purposes: 0c268fb

Owner

ozkriff commented Jan 6, 2017

That was really silly: Fow in player_info.rs is not Fow from fow.rs, it's a distinct helper struct for smooth animation of FoW updates. Renamed it to FowInfo just in case: 7cc82ff .

I had some problems with ShowUnit that I was not able to solve with reordering of EndTurn and ShowUnit/HideUnit - when enemy comes out of the fog first comes ShowUnit event for enemy's pos in the fog, then comes MoveUnit and its handler crashes as State filters out this new unit. Fixed this problem by adding shown_unit_ids: HashSet<UnitId> field to State struct: all UnitIds from ShowUnit events go there and these Ids are not filtered out even if their Unit is in the fog:

fn is_unit_visible(&self, unit: &Unit) -> bool {
    let fow = match self.fow {
        Some(ref fow) => fow,
        None => return true,
    };
    fow.is_visible(unit) || self.shown_unit_ids.contains(&unit.id) // <-
}

Plus, there was a problem with EventHideUnitVisualizer constructor - I can easily delete the node itself (by getting its Id from scene.unit_id_to_node_id) but I can't ask unit's MapPos for creating pop up text from State anymore. So I've extracted from pick.rs and employed world_pos_to_map_pos function: ad0e956 .

let world_pos = scene.node(node_id).pos;
let map_pos = geom::world_pos_to_map_pos(world_pos);
map_text.add_text(map_pos, "lost");

Added some basic maps for testing purposes: 0c268fb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment