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

Get rid of global state #135

Open
Ghabry opened this issue Sep 18, 2020 · 0 comments
Open

Get rid of global state #135

Ghabry opened this issue Sep 18, 2020 · 0 comments
Milestone

Comments

@Ghabry
Copy link
Member

Ghabry commented Sep 18, 2020

We learned from Player that the worst you can have is global state ;). In editor calling across object boundaries doesn't happen very often, there are not thousands of interactions between each other as in a game engine.

In Editor the parent object is the Core. This is a global singleton entrypoint.
The Core owns a Project which is the currently loaded project.
Outside of the MainWindow I want to discourage any access to this global (and make it private later).

Maybe later we want to have two projects loaded at once, getting rid of accessing global data is great to make this easy to implement.
I already considered this by forwarding the lcf::rpg::Database everywhere instead of doing core()->project()->database.
Forwarding project is also fine e.g. when File Finder access is required.

Current places with core-access left:

  • Worst offender is map_scene with 110 calls (but this one has refactor pending anyway).
  • Main window does 91: This is fine, main window is at the bottom of the hierarchy.

With these removed there are 147 calls left (with map_properties and stringizer removed only 71 left):

src/core.cpp
113:    for (unsigned int i = 0; i < core().project()->database().chipsets.size();i++)
114:            if (core().project()->database().chipsets[i].ID == n_chipsetid)
116:                    m_chipset = core().project()->database().chipsets[i];

src/ui/event/event_page_widget.cpp
120:    if (switchId >= 1 && switchId <= static_cast<int>(core().project()->database().switches.size())) {
123:                                     (core().project()->database().switches[static_cast<size_t>(switchId) - 1].name));
164:            if (varId >= 1 && varId <= static_cast<int>(core().project()->database().variables.size())) {
167:                                     (core().project()->database().variables[static_cast<size_t>(varId) - 1].name)));
337:            core().beginPainting(pix);
338:            core().renderTile(10000 + static_cast<short>(m_eventPage->character_index), QRect(0,0,16,16));
339:            core().endPainting();

src/ui/other/search_dialog.cpp
63:     for (auto &v : core().project()->database().variables)
65:     for (auto &s : core().project()->database().switches)
67:     for (auto &i : core().project()->database().items)
69:     for (auto &e : core().project()->database().commonevents)
78:             map_cache = std::vector<std::shared_ptr<lcf::rpg::Map>>(core().project()->treeMap().maps.size());
244:            showResults(common_searcher(core().project()->database().commonevents));
248:            for (auto &map : core().project()->treeMap().maps)
250:                    ui->label_status->setText(QString("Parsing Map %1 / %2").arg(QString::number(map.ID + 1), QString::number(core().project()->treeMap().maps.size())));
277:            const std::shared_ptr<lcf::rpg::Map> res_map{core().project()->loadMap(mapID)};
305:                            auto& mapinfo = core().project()->treeMap().maps[static_cast<size_t>(mm)];

src/ui/other/rtp_path_dialog.cpp
29:     ui->lineRtpPath->setText(core().rtpPath(""));
51:     core().setRtpDir(ui->lineRtpPath->text());

src/ui/other/run_game_dialog.cpp
97:     for (size_t i = 1; i < core().project()->treeMap().maps.size(); i++)
99:             if (core().project()->treeMap().maps[i].type == 1)
101:                    ui->comboMapId->addItem(ToQString(core().project()->treeMap().maps[i].name),
102:                                                                    core().project()->treeMap().maps[i].ID);
109:    for (size_t i = 1; i < core().project()->database().troops.size(); i++)
110:            ui->comboTroop->addItem(ToQString(core().project()->database().troops[i].name));
112:    bool auto_placement = static_cast<bool>(core().project()->database().battlecommands.placement);
122:    for (size_t i = 0; i < core().project()->database().terrains.size(); i++)
123:            ui->comboCustomFormation->addItem(ToQString(core().project()->database().terrains[i].name));
125:    battletest_data = core().project()->database().system.battletest_data;

src/ui/database/actor_delegate.cpp
34:     for (size_t i = 0; i < core().project()->database().actors.size(); i++)
35:             editor->addItem(ToQString(core().project()->database().actors[i].name), core().project()->database().actors[i].ID);
49:     model->setData(index, ToQString(core().project()->database().actors[static_cast<size_t>(id)-1].name));

src/ui/database/database_dialog.cpp
30:     m_data(core().project()->database())
80:     for (unsigned int i = 0; i < core().project()->database().actors.size(); i++)
83:                                                                .arg(core().project()->database().actors[i].name.c_str()));
176:                    core().project()->saveDatabase();

src/ui/common/faceset_picker_dialog.cpp
31:     QDir dir(core().project()->findFile(FACESET));
45:     dir = QDir(core().rtpPath(FACESET));

src/ui/common/charset_picker_dialog.cpp
40:             core().beginPainting(pix);
42:                     core().renderTile(10000+i, QRect((i%6)*16,i/6*16,16,16));
43:             core().endPainting();
51:     QDir dir(core().project()->findFile(CHARSET));
65:     dir = QDir(core().rtpPath(CHARSET));

src/ui/common/encounter_delegate.cpp
34:     for (size_t i = 0; i < core().project()->database().troops.size(); i++)
35:             editor->addItem(ToQString(core().project()->database().troops[i].name));
49:     model->setData(index, ToQString(core().project()->database().troops[static_cast<size_t>(id)-1].name));

src/ui/common/palette_scene.cpp
47:     if (core().layer() == Core::LOWER)
57:     m_tiles->graphicsEffect()->setEnabled(core().layer() != Core::LOWER);
62:     if (core().chipsetIsNull())
80:     core().beginPainting(m_lowerTiles);
84:             core().renderTile(core().translate(terrain_id,15), rect);
86:     core().renderTile(core().translate(2,0,240), QRect(64,32,32,32));
87:     core().endPainting();
88:     core().beginPainting(m_upperTiles);
92:             core().renderTile(core().translate(terrain_id+162), rect);
94:     core().endPainting();
180:            core().setSelection(sel, 1, 1);
187:                            if (core().layer() == Core::LOWER)
188:                                    sel.push_back(core().translate(_x+_y*6, SAMPLE));
190:                                    sel.push_back(core().translate(_x+_y*6+162, SAMPLE));
191:            core().setSelection(sel, w, h);

src/ui/common/faceset_item.cpp
30:     m_pix = ImageLoader::Load(core().project()->findFile(FACESET, n_pixName, FileFinder::FileType::Image));
32:             m_pix = QPixmap(core().rtpPath(FACESET,n_pixName));

src/ui/common/charset_item.cpp
35:     m_pix = ImageLoader::Load(core().project()->findFile(CHARSET, n_pixName, FileFinder::FileType::Image));
37:             m_pix = ImageLoader::Load(core().rtpPath(CHARSET, n_pixName));
39:             m_pix = core().createDummyPixmap(288,256);

src/ui/common/battle_animation_item.cpp
55:             m_pix = QPixmap(core().project()->findFile(dir, pixName, FileFinder::FileType::Image));
57:                     m_pix = QPixmap(core().rtpPath(dir, pixName));

src/ui/maptree/map_properties_dialog.cpp
33:             m_generatorLowerLayer.push_back(core().translate(terrain, UP+DOWN+LEFT+RIGHT));
51:     for (int i = 0; i < static_cast<int>(core().project()->database().chipsets.size()); i++)
52:             ui->comboTileset->addItem(ToQString(core().project()->database().chipsets[static_cast<size_t>(i)].name), i+1);
93:             item->setData(Qt::DisplayRole, ToQString(core().project()->database().troops[static_cast<size_t>(info.encounters[static_cast<size_t>(i)].troop_id)-1].name));
127:    core().beginPainting(pix);
128:    core().renderTile(map.generator_tile_ids[0], QRect(0,0,32,32));
129:    core().endPainting();
133:    core().beginPainting(pix);
134:    core().renderTile(map.generator_tile_ids[1], QRect(0,0,32,32));
135:    core().endPainting();
139:    core().beginPainting(pix);
140:    core().renderTile(map.generator_tile_ids[2], QRect(0,0,32,32));
141:    core().endPainting();
145:    core().beginPainting(pix);
146:    core().renderTile(map.generator_tile_ids[3], QRect(0,0,32,32));
147:    core().endPainting();
151:    core().beginPainting(pix);
152:    core().renderTile(map.generator_tile_ids[4], QRect(0,0,32,32));
153:    core().endPainting();
157:    core().beginPainting(pix);
158:    core().renderTile(map.generator_tile_ids[5], QRect(0,0,32,32));
159:    core().endPainting();
164:    core().beginPainting(pix);
165:    core().renderTile(map.generator_tile_ids[6], QRect(0,0,32,32));
166:    core().renderTile(map.generator_tile_ids[7], QRect(32,0,32,32));
167:    core().renderTile(map.generator_tile_ids[8], QRect(0,32,32,32));
168:    core().renderTile(map.generator_tile_ids[9], QRect(32,32,32,32));
169:    core().endPainting();
173:    core().beginPainting(pix);
174:    core().renderTile(map.generator_tile_ids[10], QRect(0,0,32,32));
175:    core().renderTile(map.generator_tile_ids[11], QRect(32,0,32,32));
176:    core().renderTile(map.generator_tile_ids[12], QRect(0,32,32,32));
177:    core().renderTile(map.generator_tile_ids[13], QRect(32,32,32,32));
178:    core().endPainting();
182:    core().beginPainting(pix);
183:    core().renderTile(map.generator_tile_ids[14], QRect(0,0,32,32));
184:    core().renderTile(map.generator_tile_ids[15], QRect(32,0,32,32));
185:    core().renderTile(map.generator_tile_ids[16], QRect(0,32,32,32));
186:    core().renderTile(map.generator_tile_ids[17], QRect(32,32,32,32));
187:    core().endPainting();
192:            pix = QPixmap(core().project()->findFile(PANORAMA, ToQString(map.parallax_name), FileFinder::FileType::Image));
194:                    pix = QPixmap(core().rtpPath(PANORAMA,ToQString(map.parallax_name)));

src/stringizer.cpp
1216:           if (id < 1 || id > static_cast<int>(core().project()->database().variables.size()))
1218:           return ToQString(core().project()->database().variables[static_cast<size_t>(id)-1].name);
1223:           if (id < 1 || id > static_cast<int>(core().project()->database().switches.size()))
1225:           return ToQString(core().project()->database().switches[static_cast<size_t>(id) - 1].name);
1230:           if (id < 1 || id > static_cast<int>(core().project()->database().items.size()))
1232:           return ToQString(core().project()->database().items[static_cast<size_t>(id)-1].name);
1237:           if (id < 1 || id > static_cast<int>(core().project()->database().actors.size()))
1239:           return ToQString(core().project()->database().actors[static_cast<size_t>(id)-1].name);
1244:           if (id < 1 || id > static_cast<int>(core().project()->database().classes.size()))
1246:           return ToQString(core().project()->database().classes[static_cast<size_t>(id)-1].name);
1251:           if (id < 1 || id > static_cast<int>(core().project()->database().states.size()))
1253:           return ToQString(core().project()->database().states[static_cast<size_t>(id)-1].name);
1258:           if (id < 1 || id > static_cast<int>(core().project()->database().skills.size()))
1260:           return ToQString(core().project()->database().skills[static_cast<size_t>(id)-1].name);
1265:           if (id < 1 || id > static_cast<int>(core().project()->database().battlecommands.commands.size()))
1267:           return ToQString(core().project()->database().battlecommands.commands[static_cast<size_t>(id)-1].name);
1273:           if (id < 1 || id > static_cast<int>(core().project()->database().animations.size()))
1275:           return ToQString(core().project()->database().animations[static_cast<size_t>(id)-1].name);
1279:           if (id < 1 || id > static_cast<int>(core().project()->database().states.size()))
1281:           return ToQString(core().project()->database().states[static_cast<size_t>(id)-1].name);
1286:           if (!core().currentMapEvent(id))
1288:           return ToQString(core().currentMapEvent(id)->name);
1293:           if (id < 1 || id > static_cast<int>(core().project()->database().commonevents.size()))
1295:           return ToQString(core().project()->database().commonevents[static_cast<size_t>(id)-1].name);
1300:           if (id < 1 || id > static_cast<int>(core().project()->database().troops.size()))
1302:           return ToQString(core().project()->database().troops[static_cast<size_t>(id)-1].name);
1307:           if (id < 1 || id > static_cast<int>(core().project()->database().terrains.size()))
1309:           return ToQString(core().project()->database().terrains[static_cast<size_t>(id)-1].name);
1314:           if (id < 1 || id > static_cast<int>(core().project()->treeMap().maps.size()))
1316:           for (unsigned i = 0; i < core().project()->treeMap().maps.size(); i++)
1317:                   if (core().project()->treeMap().maps[i].ID == id)
1318:                           return ToQString(core().project()->treeMap().maps[i].name);

src/model/enemy.cpp
32:     QPixmap monster = ImageLoader::Load(core().project()->findFile("Monster", ToQString(data().battler_name), FileFinder::FileType::Image));

src/model/actor.cpp
34:     QPixmap faceSet = ImageLoader::Load(core().project()->findFile("FaceSet", ToQString(actor.face_name), FileFinder::FileType::Image));
@Ghabry Ghabry added this to the 0.1 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant