{"action":"created","issue":{"url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/845","labels_url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/845/labels{/name}","comments_url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/845/comments","events_url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/845/events","html_url":"https://github.com/SpongePowered/SpongeAPI/issues/845","id":107351457,"number":845,"title":"Consistent providing of global access","user":{"login":"TheRaspPie","id":12686881,"avatar_url":"https://avatars.githubusercontent.com/u/12686881?v=3","gravatar_id":"","url":"https://api.github.com/users/TheRaspPie","html_url":"https://github.com/TheRaspPie","followers_url":"https://api.github.com/users/TheRaspPie/followers","following_url":"https://api.github.com/users/TheRaspPie/following{/other_user}","gists_url":"https://api.github.com/users/TheRaspPie/gists{/gist_id}","starred_url":"https://api.github.com/users/TheRaspPie/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TheRaspPie/subscriptions","organizations_url":"https://api.github.com/users/TheRaspPie/orgs","repos_url":"https://api.github.com/users/TheRaspPie/repos","events_url":"https://api.github.com/users/TheRaspPie/events{/privacy}","received_events_url":"https://api.github.com/users/TheRaspPie/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-09-19T19:17:02Z","updated_at":"2015-09-20T19:04:03Z","closed_at":null,"body":"# Why is global state bad?\r\n\r\nIn SpongeAPI there's no concept of global access to the `Server` reference like in Bukkit.\r\nInstead, we use Dependency Injection and let the injector inject the dependencies we need into the constructor or a field.\r\nNormally, this is a good thing - it helps keeping the code clean and separates different parts - if it is done consistently.\r\n\r\nIn the API, there are many places where this concept was broken in favor of usability:\r\nFor instance look at the `MessageSinks` class. There are two different ways one could get the currently logged in players.\r\n\r\nHere you need a `Server` reference somehow, i.e. by calling `Game#getServer`.\r\n```java\r\nList<Player> players = server.getOnlinePlayers();\r\n```\r\n\r\nHere you don't need one:\r\n```java\r\n@SuppressWarning(\"unchecked\")\r\nList<Player> players = (List<Player>) MessageSinks.toAllPlayers().getRecipients();\r\n```\r\n\r\nOf course the second solution is pretty ugly, but it works!\r\nThe first solution requires the `Server` context and the second one does not.\r\nTherefore the `MessageSinks` class must hold any static state which is bad for reasoning in a multi-threaded environment.\r\n\r\n# The situation with Minecraft\r\n\r\nAs you noticed, I said <b>multithreaded</b> environment. But Minecraft still lives in its single-threaded world in which we don't have to think about race conditions or deadlocks.\r\n\r\nSo my question is:\r\nWhy do we do the half thing and let people pass the `Game` object to every new listener?\r\nIf even many classes in API rely on getting initialized by the server implementation through reflection, we can make the `Game` object - and other objects that are always there - globally available.\r\n\r\nBukkit had its `Bukkit` class which provided the static `getServer()` method to get the current running server instance and static equivalents to all methods on `Server` for convenient access.\r\n\r\nAnd to be honest: The developers will get tired of passing objects anyway and create their own singleton in main class.\r\n\r\nThis is not particularly bad for small plugins which can be rewritten in hours or days.\r\nOf course, big plugins like WorldGuard may want to pass the objects around to get better decoupling, but it should be the plugin's own decision.\r\nWe shouldn't force them to not use global state, particularly because we also use singletons in the implementation ourselves.\r\n\r\nEDIT: The other option would of course be to remove all global state completely and fully rely on Dependency Injection. In the end, the decision is made by @gabizou anyway."},"comment":{"url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/comments/141821432","html_url":"https://github.com/SpongePowered/SpongeAPI/issues/845#issuecomment-141821432","issue_url":"https://api.github.com/repos/SpongePowered/SpongeAPI/issues/845","id":141821432,"user":{"login":"ST-DDT","id":1579362,"avatar_url":"https://avatars.githubusercontent.com/u/1579362?v=3","gravatar_id":"","url":"https://api.github.com/users/ST-DDT","html_url":"https://github.com/ST-DDT","followers_url":"https://api.github.com/users/ST-DDT/followers","following_url":"https://api.github.com/users/ST-DDT/following{/other_user}","gists_url":"https://api.github.com/users/ST-DDT/gists{/gist_id}","starred_url":"https://api.github.com/users/ST-DDT/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ST-DDT/subscriptions","organizations_url":"https://api.github.com/users/ST-DDT/orgs","repos_url":"https://api.github.com/users/ST-DDT/repos","events_url":"https://api.github.com/users/ST-DDT/events{/privacy}","received_events_url":"https://api.github.com/users/ST-DDT/received_events","type":"User","site_admin":false},"created_at":"2015-09-20T19:04:03Z","updated_at":"2015-09-20T19:04:03Z","body":"You kind of need that method to allow Mods/OPs to eavedrop into conversations that may run privately.\r\nOr at least you need to be able to remove or add a certain player to that (copied) sink.\r\n\r\nFor example:\r\nBadGuy: `/pm AnotherPlayer You are an ****` \r\nthe server/mod may want to punish the BadGuy for sending this\r\n````java\r\n@Listener\r\npublic void onChat(ChatEvent) {\r\n    if (!event.getSink().getRecipients().contains(adminPlayer)) {\r\n        adminPlayer.sendMessage(event.getMessage());\r\n    }\r\n}\r\n````"}}