You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Internally, we already have the following code, which shows that I had initially started to add support for true unlimited re-entries, but I half-assed it.
impl From<crate::structure::ReEntryInfo> for ReEntryInfo {
fn from(value: crate::structure::ReEntryInfo) -> Self {
Self {
last_re_entry_round: value.last_re_entry_level.into(),
re_entries: value.max_n_re_entries.unwrap_or_else(|| {
log::warn!("unlimited re-entries not well supported");
NonZeroU8::new(u8::MAX).unwrap()
}),
}
}
}
The above is super embarrassing, because not only do we have two different ReEntryInfo types, one uses max_n_re_entries as an Option<u8> where None represents "there is no max", i.e. unlimited, but the other is simply a u8. That's a recipe for disaster and that's largely what bit us yesterday (#1470).
The fix isn't hard, per-se, and rust will hold my hand while I do it, but I am not marking this high priority, because I'm not convinced that I should do this for the remaining WYWAB events, when I can do something else quicker, first (#1471).
The text was updated successfully, but these errors were encountered:
ctm
added
chore
Maintenance or other non-bug, non-feature
easy
Trivial to do (even when tired!) and semi-worthwhile
labels
Jul 16, 2024
FInish support for unlimited re-entries.
Internally, we already have the following code, which shows that I had initially started to add support for true unlimited re-entries, but I half-assed it.
The above is super embarrassing, because not only do we have two different
ReEntryInfo
types, one usesmax_n_re_entries
as anOption<u8>
whereNone
represents "there is no max", i.e. unlimited, but the other is simply au8
. That's a recipe for disaster and that's largely what bit us yesterday (#1470).The fix isn't hard, per-se, and rust will hold my hand while I do it, but I am not marking this
high priority
, because I'm not convinced that I should do this for the remaining WYWAB events, when I can do something else quicker, first (#1471).The text was updated successfully, but these errors were encountered: