Conversation
Franziska-Mueller
left a comment
There was a problem hiding this comment.
please merge develop and follow the new format for StateData and explicit read/write access via .get() / .mut()
…05-update-contract-RANDOM # Conflicts: # src/contracts/Random.h
|
#783 (review) |
|
|
||
| struct StateData | ||
| { | ||
| uint64 earnedAmount; |
There was a problem hiding this comment.
you are not using the fee and these amounts anywhere in the code
There was a problem hiding this comment.
--CFB: SC isn't finished, only half of it is coded to test in isolation, hence earnedAmount isn't touched
| locals.collateralTierFlags |= (1 << state.get().collateralTiers.get(locals.stream * 1365 + locals.i)); | ||
|
|
||
| state.mut().providers.set(locals.stream * 1365 + locals.i, | ||
| state.get().providers.get(locals.stream * 1365 + state.get().populations.get(locals.stream))); |
There was a problem hiding this comment.
I am not sure about the logic here and many places in the code, we access providers.get(locals.stream * 1365 + state.get().populations.get(locals.stream))) aka base_index + population in which I think base_index + population - 1 is more reasonable . Is this intentional ?
I saw we usually loop with below,
for (locals.i = 0; locals.i < state.get().populations.get(locals.stream); locals.i++)
{
if (state.get().revealOrCommitFlags.get(locals.stream * 1365 + locals.i))
{
...
There was a problem hiding this comment.
--CFB:
Array of 4096 elements is split into these chunks:
1 unused element.
1365 elements of steam 0
1365 elements of steam 1
1365 elements of steam 2
|
|
||
| for (locals.i = 0; locals.i < 10; locals.i++) | ||
| { | ||
| if (locals.collateralTierFlags & (1 << state.get().collateralTiers.get(locals.stream * 1365 + locals.i))) |
There was a problem hiding this comment.
This locals.i is the tier index from 0-9, state.get().collateralTiers.get(locals.stream * 1365 + locals.i)) is used for getting tier of a habitat of population, so should it be locals.collateralTierFlags & (1 << locals.i) ?
Updaet from CFB