Fixes #82 crash on startup when no project is previously selected. #93
Conversation
… selected. This bug was introduced in shpaass@8f1e04e by changing `struct RecentProject` to `class ProjectDefinition`. By making this type a reference, the default value is now null, instead of an object with all fields set to default, which leads to crashes when it is dereferenced. This fix makes the SetProject() function accept null as an arugment and then configures all variables as they would have been before. This continues to run the intialization code. Alternatively, we could skip the call to SetProject() entirely -- with my limited understanding of the code base, I don't know if that's safe or not.
jeady
changed the title
|
Ah I see now that #89 is already approved, I thought it required more review. In that case, happy to revert this PR. |
|
@jeady Thank you for your contribution. And yeah, @veger is right, I wanted to wait on @have-fun-was-taken, since although I have merging rights, they are the owner of the repo. But this is small enough that I dare to take a shortcut 😉 There was some discussion about the entire thing on Discord. "Dorus" and "Just Monika" raised some valid concern about the semantics in the current implementation. They mentioned that it would make more sense to make sure that Typically, I would argue that the So I decided to merge your PR, which hardens |
Fixes #82
This bug was introduced in 8f1e04e by changing
struct RecentProjecttoclass ProjectDefinition. By making this type a reference, the default value is now null, instead of an object with all fields set to default, which leads to crashes when it is dereferenced.This fix makes the SetProject() function accept null as an arugment and then configures all variables as they would have been before. This continues to run the intialization code. Alternatively, we could skip the call to SetProject() entirely -- with my limited understanding of the code base, I don't know if that's safe or not. I don't feel strongly about the approach, happy to do either.
#89 also fixes this by skipping the call to SetProject() if project is null, but I think it makes sense to decouple this quick bug fix from the rest of the feature work there.