General improvements

<
>
November 26, 2021

I spent the last few days on general improvements, mainly of the code base.

Module generation error handling

The player is now notified why the Module generation failed, instead of just a generic message.
The generation itself also became more restrictive: Source and Target have to be placed at the edge of a Blueprint.

Custom unwrap

In Rust one tries to have very explicit error handling via Result or Option .
But there’s cases where the program either can’t recover from failure, or cases that simply can’t happen.
In that case one can e.g. use .unwrap() .
But if one wants to quickly prototype things, using .unwrap() more often might speed up development.
Those .unwrap() calls should be removed at a later point in time.
But it now becomes impossible to tell the ‘prototype’ .unwrap() calls apart from the correct ones.
I therefore added my own aliases of .unwrap() to mark the ‘correct’ ones and also document why I consider them correct.

pub trait UnwrapExt<T> {
    // Using static data, can 'never' fail
    fn unwrap_static(self) -> T;

    // Condition in which it would fail was already checked
    fn unwrap_condition(self) -> T;

    // Would mean there's a huge logic flaw, should not fail due to runtime
    fn unwrap_logic(self) -> T;
}

impl<T> UnwrapExt<T> for Option<T> {
    fn unwrap_static(self) -> T {
        self.unwrap()
    }
    fn unwrap_condition(self) -> T {
        self.unwrap()
    }
    fn unwrap_logic(self) -> T {
        self.unwrap()
    }
}

impl<T, E> UnwrapExt<T> for std::result::Result<T, E>
where
    E: std::fmt::Debug,
{
    fn unwrap_static(self) -> T {
        self.unwrap()
    }
    fn unwrap_condition(self) -> T {
        self.unwrap()
    }
    fn unwrap_logic(self) -> T {
        self.unwrap()
    }
}

Swap logic for savegames

Instead of just overwriting the previous savegame, I now save to a temporary file which is then renamed to the actual name.
This prevents data loss due to anything going wrong while writing to disk.

Replace logic for Structures

For the simulation and player interaction Structures have to be mutated.
Up until now this was implemented by first removing the Structure, changing it accordingly and adding it back.
This ensures that e.g. tracking of occupied positions is always correct, but this is also very inefficient.
But simply allowing the mutation of Structures might cause bugs, since other containers won’t get updated.
For example in case of a Planet, the positions container would not be updated correctly if a Structure was replaced by one of different size.

pub struct Planet {
    ...
    structures: BTreeMap<StructureID, Structure>,
    positions: BTreeMap<Pos, (IsAnchorPoint, StructureID)>,
    ...

I therefore added a helper which allows direct replacement only if the size remained the same:

fn replace_with_of_same_size(
    &mut self,
    module_store: &dyn ModuleStore,
    pos: &PosAnchor,
    structure: Structure,
) -> AddResult {
    if let Some(old) = self
        .positions
        .get(&pos.0)
        .and_then(|(_, id)| self.structures.get_mut(id))
    {
        if old.size(module_store) != structure.size(module_store) {
            Err(AddError::SizeMismatch)
        } else {
            *old = structure;
            Ok(())
        }
    } else {
        Err(AddError::NoStructureAtPos)
    }
}

Keep Structures packed in memory

As seen in the code snippet above, Structures were kept in

structures: BTreeMap<StructureID, Structure>,

While this makes many operations easy to implement, it’s also very performance and memory inefficient.
Operations such as Add and Remove are very easy and efficient to implement, while iteration speed and cache locality suffer.
Add or Remove operations only happen when the player performs a manual action. Read access to the Structures happens multiple times per tick, with 60 ticks per second. It’s therefore very important to optimize for the read operations.
Hence I’m now storing Structures flat in a vector. Since some of the operations now require quite some code and are required both for the Planet and Blueprint, I introduced

pub struct PositionedStructures {
    positions: BTreeMap<Pos, (IsAnchorPoint, StructureID)>,
    structures: Vec<Structure>,
}

to be re-used.

Rename Target to Sink

I think the new name is clearer. I name this change here to avoid confusion in the future when I use the term Sink.