Skip to content
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

Bugfix event map mismatch #40

Closed
wants to merge 4 commits into from

Conversation

wlfrdssn
Copy link
Contributor

Sorry I completly messed up with github. I hope I got it right this time.

The first time you use O-Scout MapFilename will not be set when you open a map. NewEvent will not set the MapFilename. I suggest an if statement that checks if the MapFilename is set otherwise it will be set when loading map.

Copy link

vercel bot commented Jan 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
o-scout ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 24, 2025 8:21pm

@perliedman
Copy link
Owner

Nice catch! Hadn't notice this at all (probably since I always have a saved event).

Looking at the code, it looks like handling this like already done in MapSection should make the code more uniform: selecting a new map from the sidebar is done using this code:

          <SelectMap
            component={<DropdownItem>Select new map...</DropdownItem>}
            onMapLoaded={setMap}
          />

So setMap (which is the same function as setEventMap in your code) is called after a map has been loaded. In StartScreen, we could make the corresponding change, with the additional check to see if a map is already set:

              <SelectMap
                type="primary"
                className="text-xl w-48 h-16"
                onMapLoaded={(mapFile, mapFilename) => {
                  if (!event.mapFilename) {
                    event.actions.event.setMap(mapFile, mapFilename);
                  }
                }}
              >

@wlfrdssn
Copy link
Contributor Author

wlfrdssn commented Feb 9, 2025

I have a hard time with anything involves zustand store. A lot of trial and error. I think I understand your point and if I understand you correctly, the only thing we need is a callback that checks whether a mapfilename is set, if not we can set the name using event.actions.event.setMap(mapFile, mapFilename);

@perliedman
Copy link
Owner

Yeah, I understand there's a learning curve to zustand (or really any React state handling). To better understand why this complexity exists, just think about undo/redo which sort of magically "just works" as long as you follow the zustand pattern.

@perliedman perliedman closed this in 24287ea Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants